Skip to content
This repository was archived by the owner on Jun 5, 2025. It is now read-only.

Add a FIM pipeline to Providers#102

Merged
aponcedeleonch merged 9 commits intomainfrom
add-fim-pipeline
Nov 28, 2024
Merged

Add a FIM pipeline to Providers#102
aponcedeleonch merged 9 commits intomainfrom
add-fim-pipeline

Conversation

@aponcedeleonch
Copy link
Copy Markdown
Member

Related: #87, #43

The PR adds a FIM pipeline independent from chat completion pipeline.
It could still be faulty since we need:

  • Message normalizer. We now expect all messages to have the key messages. However, there are incoming messages with prompt.
  • Secreets detector. There's the skeleton of a class called SecretAnalyzer that is meant to analyze the messages and return a warning if it detected a secret.

@aponcedeleonch
Copy link
Copy Markdown
Member Author

Tested with Anthropic since we don't need Message Normalizer for that one:

Screenshot 2024-11-27 at 15 20 50

Comment on lines +28 to +30
model_in_request = completion_request['model']
if not model_in_request.startswith('anthropic/'):
completion_request['model'] = f'anthropic/{model_in_request}'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this block? I don't think any anthropic models start with anthropic. Either way, why do we change the model to have the anthropic prefix?

Copy link
Copy Markdown
Member Author

@aponcedeleonch aponcedeleonch Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to LiteLLM docs if we prepepnd anthropic/ to the model name it will always force the request to anthropic. Otherwise, it will try to map the model name to one that they have in their registry. However, it accepts most names. I tried with claude-3-5-sonnet-latest and didn't accept it as a valid anthropic model. Hence, I made this change. But agree, maybe this is being too careful and can be removed.

Doc Reference

@aponcedeleonch aponcedeleonch marked this pull request as draft November 27, 2024 16:19
@aponcedeleonch
Copy link
Copy Markdown
Member Author

aponcedeleonch commented Nov 27, 2024

Drafting because as it is FIM is working for Anthropic but will not work with the other providers that we have: OpenAI and llama.cpp. I will wait to rebase the changes by Jakub to get things working for all providers.

@ptelang
Copy link
Copy Markdown
Contributor

ptelang commented Nov 27, 2024

Looks good!

jhrozek and others added 5 commits November 28, 2024 10:02
This adds a pipeline processing before the completion is ran where
the request is either change or can be shortcut. This pipeline consists
of steps, for now we implement a single step `CodegateVersion` that
responds with the codegate version if the verbatim `codegate-version`
string is found in the input.

The pipeline also passes along a context, for now that is unused but I
thought this would be where we store extracted code snippets etc.

To avoid import loops, we also move the `BaseCompletionHandler` class to
a new `completion` package.

Since the shortcut replies are more or less simple strings, we add yet
another package `providers/formatting` whose responsibility is to
convert the string returned by the shortcut response to the format
expected by the client, meaning either a reply or a stream of replies in
the LLM-specific format. We use the `BaseCompletionHandler` as a way to
convert to the LLM-specific format.
Related: #87, #43

The PR adds a FIM pipeline independent from chat completion pipeline.
It could still be faulty since we need:
- Message normalizer. We now expect all messages to have the key `messages`. However, there are incoming messages with `prompt`.
- Secreets detector. There's the skeleton of a class called SecretAnalyzer that is meant to analyze the messages and return a warning if it detected a secret.
@aponcedeleonch aponcedeleonch marked this pull request as ready for review November 28, 2024 10:49
@lukehinds
Copy link
Copy Markdown

Don't merge yet. You should use the go port that uses the large signatures list we have

I will have this up in a minute

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants