Add the ability to use LLM Providers from the Arch config #112
Conversation
Signed-off-by: José Ulises Niño Rivera <[email protected]>
Signed-off-by: José Ulises Niño Rivera <[email protected]>
Signed-off-by: José Ulises Niño Rivera <[email protected]>
Signed-off-by: José Ulises Niño Rivera <[email protected]>
Signed-off-by: José Ulises Niño Rivera <[email protected]>
Signed-off-by: José Ulises Niño Rivera <[email protected]>
arch/config_generator.py
Outdated
| def add_secret_key_to_llm_providers(config_yaml) : | ||
| llm_providers = [] | ||
| for llm_provider in config_yaml.get("llm_providers", []): | ||
| if llm_provider['access_key'] == "mistral_access_key": |
There was a problem hiding this comment.
mistral_access_key should be $MISTRAL_ACCESS_KEY, same for openai provider
There was a problem hiding this comment.
and should only modify access keys if OPENAI_API_KEY is not false
There was a problem hiding this comment.
and should only modify access keys if OPENAI_API_KEY is not false
@adilhafeez. This was actually part of a larger question I had: what should behavior be if access keys are not correctly set. Should we panic and abort in initialization?
There was a problem hiding this comment.
Just send the request upstream and let it fail - there may be some providers that doesn't require access keys (e.g. locally hosted models). So setting keys should be optional in my opinion.
| pub fn api_key_header(&self) -> &str { | ||
| self.api_key_header | ||
| } | ||
| #[derive(thiserror::Error, Debug)] |
There was a problem hiding this comment.
will use these in my errors too ❤️
There was a problem hiding this comment.
Yeah, this crate is excellent. I didn't want to bust it too soon. But with this I felt it was necessary.
Check out @dtolnay, a lot of very useful crates started with him. serde too
| match llm_providers.default { | ||
| Some(_) => return Err(LlmProvidersNewError::MoreThanOneDefault), | ||
| None => llm_providers.default = Some(Rc::clone(&llm_provider)), | ||
| } |
| ) -> Rc<LlmProvider> { | ||
| let maybe_provider = provider_hint.and_then(|hint| match hint { | ||
| ProviderHint::Default => llm_providers.default(), | ||
| // FIXME: should a non-existent name in the hint be more explicit? i.e, return a BAD_REQUEST? |
There was a problem hiding this comment.
should be bad req in that case
| if prompt_guard_resp.jailbreak_verdict.is_some() | ||
| && prompt_guard_resp.jailbreak_verdict.unwrap() | ||
| { | ||
| if prompt_guard_resp.jailbreak_verdict.unwrap_or_default() { |
| let msg = (*self.prompt_guards) | ||
| .as_ref() | ||
| .and_then(|pg| pg.jailbreak_on_exception_message()) | ||
| .unwrap_or("Jailbreak detected. Please refrain from discussing jailbreaking."); | ||
| return self.send_server_error(msg.to_string(), Some(StatusCode::BAD_REQUEST)); |
Signed-off-by: José Ulises Niño Rivera <[email protected]>
|
@adilhafeez I get this in the function calling demo: is this correct? |
Signed-off-by: José Ulises Niño Rivera <[email protected]>
| access_key: $MISTRAL_API_KEY | ||
| model: "mistral-8x7b" | ||
|
|
||
| - name: "MistralLocal7b" | ||
| provider: "local" |
There was a problem hiding this comment.
I noticed that the schema, or any other part of the code deals with local providers. This PR does not enable that use case either.
I think the endpoint field and the provider field should be an enum making the choice mutually exclusive? Lmk what you think, and I can iterate in a subsequent PR.
There was a problem hiding this comment.
yes that would be nice cleanup
Yes model makes mistake sometimes. Try asking "what is the weather in chicago?" |
No description provided.