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: Leading whitespace chunks break partial parser (structured outputs) #1213

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

Conversation

tillkolter
Copy link

@tillkolter tillkolter commented Dec 5, 2024

  • I understand that this repository is auto-generated and my pull request may not be merged

Changes being requested

Although gpt-4o-mini-2024-07-18 in general supports structured outputs, we observed after integration that the model responds quite regularly with leading newline chunks (while the overall response is valid JSON)

Since the partial parser does not allow (trimmed) empty strings – which is also reasonable – the only reasonable thing to handle such case is to handle this case before the parser is tried and default to a null value instead.

Additional context & links

Example
Fingerprint: fp_0705bf87c0
Request ID: chatcmpl-Ab7XYB0hNxuJawO9gM0c4Doy0GTQP

Beginning of example output stream

data: {"id":"chatcmpl-Ab7XYB0hNxuJawO9gM0c4Doy0GTQP","object":"chat.completion.chunk","created":1733410484,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_0705bf87c0","choices":[{"index":0,"delta":{"role":"assistant","content":"","refusal":null},"logprobs":null,"finish_reason":null}],"usage":null}

data: {"id":"chatcmpl-Ab7XYB0hNxuJawO9gM0c4Doy0GTQP","object":"chat.completion.chunk","created":1733410484,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_0705bf87c0","choices":[{"index":0,"delta":{"content":"\n\n"},"logprobs":null,"finish_reason":null}],"usage":null}

data: {"id":"chatcmpl-Ab7XYB0hNxuJawO9gM0c4Doy0GTQP","object":"chat.completion.chunk","created":1733410484,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_0705bf87c0","choices":[{"index":0,"delta":{"content":"{\""},"logprobs":null,"finish_reason":null}],"usage":null}
...

@tillkolter tillkolter requested a review from a team as a code owner December 5, 2024 15:47
@tillkolter tillkolter force-pushed the fix/leading-empty-chunks branch from 7327a60 to d23aed8 Compare December 6, 2024 06:13
@RobertCraigie
Copy link
Collaborator

Thanks for the PR, would you be able to add a test case for this change?

@tillkolter tillkolter force-pushed the fix/leading-empty-chunks branch from d23aed8 to ad3af0d Compare December 7, 2024 09:11
@tillkolter
Copy link
Author

tillkolter commented Dec 7, 2024

@RobertCraigie

I added a test case. I hope you bear with me, that I piggy backed copying/modifying one of your working snapshops and modified it according to our observation with the leading newlines on more complex input/message history.

@tillkolter tillkolter force-pushed the fix/leading-empty-chunks branch from ad3af0d to fb01944 Compare December 12, 2024 21:29
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.

2 participants