function calling fixes for together/llama-3.1 #446
Conversation
kwindla
requested review from
aconchillo,
filipi87,
LewisWolfgang and
marcus-daily
| function_call_accumulator = "<function" | ||
| else: | ||
| await self.push_frame("<" + TextFrame(chunk.choices[0].delta.content)) | ||
| elif text == '<': |
There was a problem hiding this comment.
This seems to make the assumption that < will arrive in its own chunk with nothing else included, as will the word function -- just checking if that assumption is valid?
Do the chunks here correspond to the SSE chunks from Together, and if so, is the above documented behaviour, or something we've observed empirically?
There was a problem hiding this comment.
It's not documented. But I did test really heavily and this seems to be the pattern empirically. It makes sense, given what I've seen in general and the Llama 3.1 tokenizer, I think.
My thinking is that we should implement this way, for now, and then revisit when we have a chance to regularize around @chadbailey59's ideas that there should be configurable "structured data" patterns that aren't streamed through the pipeline.
| # Function format regex. Llama 3.1 sometimes adds an extra " or space just before the | ||
| # </function> tag. This regexp just ignores the extra characters if they are there. (That's | ||
| # the [\s"]? part of the regex.) Occasionally the </function> close tag is also missing. | ||
| function_regex = r'<function=(\w+)>(.*?)<\/function>|<function=(\w+)>(.*)' |
There was a problem hiding this comment.
Is the question mark necessary in .*? ?
There was a problem hiding this comment.
Ah never mind, I assume this is to turn this into a lazy match.
|
This looks fine to me, but I will leave it to others to approve, as I have not tested function calling with Together. |
Fixes for these function call test cases:
And also extracts a function call from the middle of an output (not just the beginning).