Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Cody Gateway: handle streams with trailing newline in Gemini response#63172

Merged
abeatrix merged 1 commit into
mainfrom
bee/fix-gemini-token
Jun 8, 2024
Merged

Cody Gateway: handle streams with trailing newline in Gemini response#63172
abeatrix merged 1 commit into
mainfrom
bee/fix-gemini-token

Conversation

@abeatrix

@abeatrix abeatrix commented Jun 7, 2024

Copy link
Copy Markdown
Contributor

CONTEXT: https://sourcegraph.slack.com/archives/C05ABRRGB0B/p1717790701356599

Fix an issue where a valid Gemini response ends with new lines, causing a false alert.

image

Issue: We are seeing *errutil.leafError: no Google response found in Sentry complaining when I run a command or chat in VS Code using google as provider.

Cause: Currently the code (added my me) would skip to the last line of the stream response and determine if the response is valid or not, which could be an issue because the stream API could ends the response with an empty new line, where our current logic would fail.

Changes included in this PR:

  • Modify parseGoogleTokenUsage function to find the last non-empty line in the stream, to handle cases where the stream ends with a newline
  • Add a test case to cover the scenario where the stream ends with a newline

This change modifies the behavior of the parseGoogleTokenUsage function to handle streams with trailing newlines, which would fail even if the response is valid because Gemini adds a new line to the end of their stream.

Test plan

Make a curl command to the Gemini streaming API to confirm the response ends with a new line:

image

curl https://generativelanguage.googleapis.com/v1beta/models/gemini-1.5-flash:streamGenerateContent\?alt=sse\&key\=$GOOGLE_API_KEY \
    -H 'Content-Type: application/json' \
    -X POST \
    -d '{
      "contents": [
        {"role":"user",
         "parts":[{
           "text": "Write the first line of a story about a magic backpack."}]},
        {"role": "model",
         "parts":[{
           "text": "In the bustling city of Meadow brook, lived a young girl named Sophie. She was a bright and curious soul with an imaginative mind."}]},
        {"role": "user",
         "parts":[{
           "text": "Can you set it in a quiet village in 1600s France?"}]},
      ]
    }' 2> /dev/null

Copied the response to the test file and confirmed our current test would fail with the same error message:

image

This is now handled by the newly added test with the actually stream response returned by calling the Gemini API.

Changelog

…esponse

- Modify `parseGoogleTokenUsage` function to find the last non-empty line in the stream, to handle cases where the stream ends with a newline
- Add a test case to cover the scenario where the stream ends with a newline

This change modifies the behavior of the `parseGoogleTokenUsage` function to handle streams with trailing newlines, which would fail even if the response is valid because Gemini adds a new line to the end of their stream.
@cla-bot cla-bot Bot added the cla-signed label Jun 7, 2024
@abeatrix abeatrix requested review from a team and chrsmith June 7, 2024 23:15
@abeatrix abeatrix enabled auto-merge (squash) June 7, 2024 23:18

@chrsmith chrsmith left a comment

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.

In the interest of getting you unblocked, if you have reproduced the issue and updated the unit tests to now catch the problem. Then let's just get this checked n.

But this parseGoogleTokenUsage function seems a little off.

Can't we just call io.ReadAll and just read it into memory, and then manipulate the data as a string? Needing to deal with scanners is usually tedious and hard to get right. (e.g. lastNonEmptyLine = line.

But I'm sure it would take a non-trivial amount of time to rewrite this to simplify things, so let's just get it fixed.

@abeatrix abeatrix merged commit d288874 into main Jun 8, 2024
@abeatrix abeatrix deleted the bee/fix-gemini-token branch June 8, 2024 22:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants