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

fix: Update OpenAIApi.cs for chunks incomplete bug #87

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SebastianBlandon
Copy link

when I received the messages by stream or chunks sometimes I received the last message incomplete, the reason was that the request.isDone() was set to false and finished receiving messages before receiving the message of finisih_reason of the OpenAI API, then I modified the logic so that it only closes the cycle when I get the stop in finish_reason that is the flag that warns me that it finished sending me chunks.

@SebastianBlandon
Copy link
Author

Solved Issues:

#59
#50
#81

@@ -122,10 +122,9 @@ public OpenAIApi(string apiKey = null, string organization = null)
foreach (string line in lines)
{
var value = line.Replace("data: ", "");

if (value.Contains("[DONE]"))
if (value.Contains("stop"))
Copy link

Choose a reason for hiding this comment

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

Since value contains the entire chunk response, wouldn't this incorrectly set isDone to true if the delta content contained the word stop? I think a better solution would be to only set isDone when the value contains [DONE]. Or explicitly see if it contains "finish_reason":"stop"

@BBrown4
Copy link

BBrown4 commented Dec 6, 2023

Edit: In fact, technically you don't even need to use isDone at all because the loop will break if the value contains [DONE] but better safe than sorry I guess?

Original:
I'd recommend this instead:

request.SendWebRequest();
var isDone = false;

do
{
    List<T> dataList = new();
    
    var lines = request.downloadHandler.text.Split('\n').Where(line => line != "").ToArray();
    
    foreach (var line in lines)
    {
        var value = line.Replace("data:", "");
        
        Debug.Log(value);
    
        if (value.Contains("[DONE]"))
        {
            isDone = true;
            onComplete?.Invoke();
            break;
        }
    
        var data = JsonConvert.DeserializeObject<T>(value);
    
        if (data?.Error != null)
        {
            var error = data.Error;
            Debug.LogError($"Error message: {error.Message}\nError type: {error.Type}\n");
        }
        else
        {
            dataList.Add(data);
        }
    }
    
    onResponse?.Invoke(dataList);
    await Task.Yield();
} while (!isDone);

@David624634
Copy link

@srcnalt It would be greatly appreciated if this pull request could be merged so that the stream feature actually works without local changes.

@jplumi
Copy link

jplumi commented Apr 3, 2024

@srcnalt please merge this pull request.

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.

None yet

4 participants