-
Notifications
You must be signed in to change notification settings - Fork 26
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
Support jsonl responses from llms #21
base: main
Are you sure you want to change the base?
Support jsonl responses from llms #21
Conversation
@@ -1,3 +1,8 @@ | |||
//go:build ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The WithProviderRegistry
function is not present in the package. Maybe it's on another branch? I had to comment this out in order to get tests to work.
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestLiveAPICallWithGenerate(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this "live" test to verify that I hadn't broken openai response parsing. I added an ollama test as well. I don't have keys to other providers, but those that do could add similar Runners.
Note, the tests are skipped if the API keys (for remote services like openai) or valid endpoints (like Ollama) are not present.
@@ -5,6 +5,7 @@ import ( | |||
"testing" | |||
|
|||
"github.com/stretchr/testify/assert" | |||
"github.com/stretchr/testify/mock" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how tests were passing without this import.
cacheInfo := map[string]interface{}{ | ||
"cache_creation_input_tokens": usage["cache_creation_input_tokens"], | ||
"cache_read_input_tokens": usage["cache_read_input_tokens"], | ||
if err := json.Unmarshal(body, &fullResponse); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the meat of the PR. If JSON parsing fails, we try parsing JSONL and getting the last entry.
This PR gracefully handles processing of JSONL responses from LLMs. Closes #20.
See comments in PR for other changes needed to get testing to work.