Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Response parsing logic fixes! #19

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

felixphixer
Copy link

@felixphixer felixphixer commented Feb 26, 2024

Update: (March 9, 2024)

Removed all the hacks!

@DJ4ddi had a better way to handle it using a library for such a case and I have incorporated their changes!


(Old) Warning: I don't think this belongs in the main, but maybe in an another branch...

This is an awesome lightweight ollama client to play with. Thank you for creating it!

Sadly, my "ollama serve" was sending fragmented JSONs and the stream reader logic in api.ts using the /api/generate api was getting confused and nothing was getting loaded in the chats.

Well since there is a bug bounty on a completely different approach using the openai API this is just a hacky solution until then for someone running into a similar issue on their local ollama server. Thought I would share this.

< bio-hazard suit needed below >

  1. 'chunk' was sometimes filled with '{...}{...}...' type responses which was breaking the JSON parse. So added an inner loop to collapse these into a single response. The fix is probably missing other cases but this mostly fixes the problems I am seeing,

  2. Also for the 'done': true with the context field In the end the reader was getting fragmented JSONs because of the size so another fix ends up reading to the end since this is the last message anyway.

@DJ4ddi
Copy link

DJ4ddi commented Mar 7, 2024

I ran into the same issue and decided to let someone else do the work for me, so I implemented a fix using the can-ndjson-stream package. I'm not going to create my own PR since I have other changes, but here's the commit for anyone interested.

@felixphixer
Copy link
Author

@DJ4ddi This looks great! Thank you! Will incorporate your fix in this PR.

@felixphixer felixphixer changed the title Response parsing logic hacks. It works! Response parsing logic fixes! Mar 9, 2024
@felixphixer
Copy link
Author

Seeing fragmented responses in the chat window when the reader gets flooded with faster model responses. So introduced a tiny rate limit to mitigate this. The actual issue might be up in the event handling layer.

@TheDevelolper
Copy link

TheDevelolper commented Apr 7, 2024

I've been running into this issue, glad I checked here first before attempting to fix. I'll take your branch as it's not been merged yet.

@HelgeSverre
Copy link
Owner

Hmmm after we swapped over to using the chat endpoint this is currently broken, if you wanna take another crack at it i'll review it more quickly next time 🙃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants