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

Conversation

@jhrozek
Copy link
Contributor

@jhrozek jhrozek commented Jan 30, 2025

We were not passing the suffix at all which was breaking the FIM
completion.

Fixes: #853

JAORMX
JAORMX previously approved these changes Jan 30, 2025
jhrozek and others added 7 commits January 30, 2025 17:05
We just blindly took the first message which could have been a system
message. The prompt is in the user message, so let's pass that.
Some models don't use FIM markers in the code, but use the suffix
top-level attribute instead. If we don't pass it, the FIM won't complete
successfully.
Signed-off-by: Radoslav Dimitrov <[email protected]>
Signed-off-by: Radoslav Dimitrov <[email protected]>
The `raw` parameter tells the LLM to never use natural language, but
just reply in the format of the message. We need to pass that to the
generate call or else we migth get garbage back to the client.
This is just useful to debug the tests
The FIM format didn't match the model it seemed. I replaced it with a
dump of a FIM message I received from Continue.
Copy link
Member

@rdimitrov rdimitrov left a comment

Choose a reason for hiding this comment

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

Nice!

@jhrozek jhrozek merged commit d81e0e5 into main Jan 31, 2025
4 checks passed
@jhrozek jhrozek deleted the ollama_fim_fix branch January 31, 2025 08:12
lukehinds pushed a commit that referenced this pull request Jan 31, 2025
* Do not send the system message, only the user message to Ollama FIM

We just blindly took the first message which could have been a system
message. The prompt is in the user message, so let's pass that.

* Pass the suffix parameter if present to the FIM generate call

Some models don't use FIM markers in the code, but use the suffix
top-level attribute instead. If we don't pass it, the FIM won't complete
successfully.

* Run make format

Signed-off-by: Radoslav Dimitrov <[email protected]>

* Fix the unit tests for ollama FIM

Signed-off-by: Radoslav Dimitrov <[email protected]>

* Pass along the `raw` parameter

The `raw` parameter tells the LLM to never use natural language, but
just reply in the format of the message. We need to pass that to the
generate call or else we migth get garbage back to the client.

* Print the full reply as debug message in integration tests

This is just useful to debug the tests

* Adjust the Ollama FIM testcase to match the model we are using

The FIM format didn't match the model it seemed. I replaced it with a
dump of a FIM message I received from Continue.

---------

Signed-off-by: Radoslav Dimitrov <[email protected]>
Co-authored-by: Radoslav Dimitrov <[email protected]>
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.

Ollama FIM doesn't work

4 participants