Centralize system message handling in adapters; add developer message support
Two goals: 1. Centralize system_instruction vs context system message resolution into the LLM adapters. This eliminates duplication between in-pipeline and out-of-band (run_inference) code paths across ~16 locations in service llm.py files. 2. Add support for "developer" role messages in conversation context, which is facilitated by the above centralization. Shared helpers on BaseLLMAdapter: - _extract_initial_system_or_developer: extracts/converts messages[0] based on role and whether system_instruction is provided - _resolve_system_instruction: warns on conflicts between system_instruction and context system messages, returns the effective instruction Developer message handling (new): - Non-OpenAI adapters: an initial "developer" message is promoted to the system instruction when no system_instruction is provided; otherwise it is converted to "user". Subsequent "developer" messages are always converted to "user". No conflict warning is emitted for developer messages (unlike "system" messages). - OpenAI adapter: "developer" messages pass through in conversation history without triggering conflict warnings. - OpenAI Responses adapter: "developer" messages are kept as "developer" role (same as "system", which is also converted to "developer" for the Responses API). Other behavior changes: - Gemini: "initial" system message detection now checks messages[0] only (previously searched anywhere in the list) - Bedrock: a lone system message is now converted to "user" instead of being extracted to an empty message list (matches existing Anthropic behavior)
This commit is contained in:
@@ -60,7 +60,9 @@ async def test_openai_run_inference_with_llm_context():
|
||||
# Verify
|
||||
assert result == "Hello! How can I help you today?"
|
||||
service.get_llm_adapter.assert_called_once()
|
||||
mock_adapter.get_llm_invocation_params.assert_called_once_with(mock_context)
|
||||
mock_adapter.get_llm_invocation_params.assert_called_once_with(
|
||||
mock_context, system_instruction=None
|
||||
)
|
||||
service._client.chat.completions.create.assert_called_once_with(
|
||||
model="gpt-4",
|
||||
stream=False,
|
||||
@@ -187,7 +189,7 @@ async def test_anthropic_run_inference_with_llm_context():
|
||||
assert result == "Hello! How can I help you today?"
|
||||
service.get_llm_adapter.assert_called_once()
|
||||
mock_adapter.get_llm_invocation_params.assert_called_once_with(
|
||||
mock_context, enable_prompt_caching=False
|
||||
mock_context, enable_prompt_caching=False, system_instruction=None
|
||||
)
|
||||
service._client.beta.messages.create.assert_called_once_with(
|
||||
model="claude-3-sonnet-20240229",
|
||||
@@ -302,7 +304,9 @@ async def test_google_run_inference_with_llm_context():
|
||||
# Verify
|
||||
assert result == "Hello! How can I help you today?"
|
||||
service.get_llm_adapter.assert_called_once()
|
||||
mock_adapter.get_llm_invocation_params.assert_called_once_with(mock_context)
|
||||
mock_adapter.get_llm_invocation_params.assert_called_once_with(
|
||||
mock_context, system_instruction=None
|
||||
)
|
||||
service._client.aio.models.generate_content.assert_called_once()
|
||||
|
||||
|
||||
@@ -421,7 +425,9 @@ async def test_aws_bedrock_run_inference_with_llm_context():
|
||||
# Verify
|
||||
assert result == "Hello! How can I help you today?"
|
||||
service.get_llm_adapter.assert_called_once()
|
||||
mock_adapter.get_llm_invocation_params.assert_called_once_with(mock_context)
|
||||
mock_adapter.get_llm_invocation_params.assert_called_once_with(
|
||||
mock_context, system_instruction=None
|
||||
)
|
||||
|
||||
# Verify the call includes configured parameters
|
||||
call_kwargs = mock_client.converse.call_args.kwargs
|
||||
@@ -543,15 +549,10 @@ async def test_openai_run_inference_system_instruction_overrides_context():
|
||||
)
|
||||
|
||||
assert result == "Response"
|
||||
call_kwargs = service._client.chat.completions.create.call_args.kwargs
|
||||
messages = call_kwargs["messages"]
|
||||
# system_instruction should be prepended as the first message
|
||||
assert messages[0] == {"role": "system", "content": "New system instruction"}
|
||||
# Original system message should still be present
|
||||
assert messages[1] == {"role": "system", "content": "Original system message"}
|
||||
# User message should still be present
|
||||
assert messages[2] == {"role": "user", "content": "Hello"}
|
||||
assert len(messages) == 3
|
||||
# Verify the adapter was called with the correct system_instruction
|
||||
mock_adapter.get_llm_invocation_params.assert_called_once_with(
|
||||
mock_context, system_instruction="New system instruction"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -608,9 +609,12 @@ async def test_anthropic_run_inference_system_instruction_overrides_context():
|
||||
result = await service.run_inference(mock_context, system_instruction="New system instruction")
|
||||
|
||||
assert result == "Response"
|
||||
call_kwargs = service._client.beta.messages.create.call_args.kwargs
|
||||
assert call_kwargs["system"] == "New system instruction"
|
||||
assert call_kwargs["messages"] == test_messages
|
||||
# Verify the adapter was called with the correct system_instruction
|
||||
mock_adapter.get_llm_invocation_params.assert_called_once_with(
|
||||
mock_context,
|
||||
enable_prompt_caching=False,
|
||||
system_instruction="New system instruction",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -665,9 +669,10 @@ async def test_google_run_inference_system_instruction_overrides_context():
|
||||
result = await service.run_inference(mock_context, system_instruction="New system instruction")
|
||||
|
||||
assert result == "Response"
|
||||
call_kwargs = service._client.aio.models.generate_content.call_args.kwargs
|
||||
config = call_kwargs["config"]
|
||||
assert config.system_instruction == "New system instruction"
|
||||
# Verify the adapter was called with the correct system_instruction
|
||||
mock_adapter.get_llm_invocation_params.assert_called_once_with(
|
||||
mock_context, system_instruction="New system instruction"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -731,9 +736,10 @@ async def test_aws_bedrock_run_inference_system_instruction_overrides_context():
|
||||
)
|
||||
|
||||
assert result == "Response"
|
||||
call_kwargs = mock_client.converse.call_args.kwargs
|
||||
assert call_kwargs["system"] == [{"text": "New system instruction"}]
|
||||
assert call_kwargs["messages"] == test_messages
|
||||
# Verify the adapter was called with the correct system_instruction
|
||||
mock_adapter.get_llm_invocation_params.assert_called_once_with(
|
||||
mock_context, system_instruction="New system instruction"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
Reference in New Issue
Block a user