RFC: config to override system prompt #912
Conversation
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
ehhuang
requested review from
ashwinb,
yanxi0830,
hardikjshah,
dltn,
raghotham,
dineshyv,
vladimirivic and
sixianyi0721
as code owners
facebook-github-bot
added
the
CLA Signed
| @json_schema_type | ||
| class ToolConfig(BaseModel): | ||
| # zero-shot tool definitions as input to the model | ||
| tools: Optional[List[ToolDefinition]] = Field(default_factory=list) |
There was a problem hiding this comment.
I feel like we could leave this at the top level so the common case (no ToolConfig) stays simpler without needing a nested parameter
| class OverrideSystemMessage(Enum): | ||
| """Config for how to override the default system prompt. | ||
|
|
||
| :cvar append: Appends the provided system message to the default system prompt: |
There was a problem hiding this comment.
by system message, you mean the instructions field in the AgentConfig right? if so, maybe making that explicit would be good
There was a problem hiding this comment.
I think this is the inference API? Lower level than Agent?
There was a problem hiding this comment.
@ehhuang sorrry you are right. big brainfart; did not realize this was for inference
| @@ -310,18 +310,49 @@ class CompletionResponseStreamChunk(BaseModel): | |||
| logprobs: Optional[List[TokenLogProbs]] = None | |||
|
|
|||
|
|
|||
| # This is an internally used class | |||
| @json_schema_type | |||
| class OverrideSystemMessage(Enum): | |||
There was a problem hiding this comment.
wondering if we could name this InstructionsOverrideBehavior?
There was a problem hiding this comment.
update: maybe call this SystemMessageBehavior or SystemMessageOverride (that's a nit)?
| response_format: Optional[ResponseFormat] = None | ||
| stream: Optional[bool] = False | ||
| logprobs: Optional[LogProbConfig] = None | ||
|
|
||
| # DEPRECATED: use tool_config instead |
There was a problem hiding this comment.
noice! I think we can add Field(deprecated="use tool_config instead") in the annotation also so it can reach the docs automatically.
|
Thanks for the comments. Addressed in #915 (sorry I think my repo was messed up so just created a new one) |
What does this PR do?
The current default system prompt for llama3.2 tends to overindex on tool calling and doesn't work well when the prompt does not require tool calling.
This PR adds an option to override the default system prompt, and organizes tool-related configs into a new config object.
Test Plan
Please describe:
Sources
Please link relevant resources if necessary.
Before submitting
Pull Request section?