Get rid of LLMContext.get_messages_for_persistent_storage().
The reason for its `system_instruction` argument was to support usage with LLMs where you might pass the system instruction as a parameter to the `LLMService` rather than specifying it in the context. But as I thought about it more I became unconvinced that the `system_instruction` argument was really beneficial: - If you specified your system instruction in your context in the first place, it'll still be there when you read messages for persistent storage - If you didn't specify your system instruction in the context and instead passed it in as an `LLMService` parameter, you most likely *don't* want it to be in the context when you read messages for persistent storage - ...and if you really really do need to inject it at the start of the context, it's quite easy to do anyway And if we remove the `system_instruction` argument from `get_messages_for_persistent_storage()`, then it's essentially just `get_messages()`.
This commit is contained in:
@@ -131,21 +131,6 @@ class LLMContext:
|
||||
)
|
||||
return filtered_messages
|
||||
|
||||
def get_messages_for_persistent_storage(
|
||||
self, system_instruction: Optional[str] = None
|
||||
) -> List[LLMContextMessage]:
|
||||
"""Get messages formatted for persistent storage.
|
||||
|
||||
Args:
|
||||
system_instruction: Optional system instruction to ensure is
|
||||
included as the first message in the returned list, if not
|
||||
already present.
|
||||
"""
|
||||
messages = copy.deepcopy(self.get_messages())
|
||||
if system_instruction and (not messages or messages[0].get("role") != "system"):
|
||||
messages.insert(0, {"role": "system", "content": system_instruction})
|
||||
return messages
|
||||
|
||||
@property
|
||||
def tools(self) -> ToolsSchema | NotGiven:
|
||||
"""Get the tools list.
|
||||
|
||||
Reference in New Issue
Block a user