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

extract_json_between_markers doesn't handle responses missing json markers #53

Open
robotdad opened this issue Aug 21, 2024 · 5 comments · May be fixed by #164
Open

extract_json_between_markers doesn't handle responses missing json markers #53

robotdad opened this issue Aug 21, 2024 · 5 comments · May be fixed by #164

Comments

@robotdad
Copy link

I found when using this with Azure OpenAI that the llm responses were coming back as valid json but extract_json_between_markers was rejecting them because it is enforcing a check for the json marker which was not in the response.

I can submit a PR to add a fallback to try parsing the output as json without those markers if you want it.

@conglu1997
Copy link
Collaborator

Hi! Sure thing if you think there would be a measurable improvement to success rate, from our observations this could happen but unclear whether this happens that often or more than other types of JSON formatting issues!

@robotdad
Copy link
Author

I encountered raw json running perform_review with gpt4o on Azure. The change in the pr worked there, but I have not run this against other models.

@conglu1997
Copy link
Collaborator

Interesting - this is quite the pathological case since we ask for chain of thought steps as well as the JSON. We didn’t observe failures in formatting more than perhaps 1% of the time, are you observing higher?

@robotdad
Copy link
Author

I only ran the perform_review with a paper using gpt4o on Azure. It was always returned as well formatted json but the method was returning none because it did not have the json markers. With this fallback mechanism it is returned as valid json.

@jli113
Copy link

jli113 commented Sep 2, 2024

I used strictjson to reformat the responses, since I'm running locally, token is no longer an issue.

@erkinalp erkinalp linked a pull request Dec 21, 2024 that will close this issue
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 a pull request may close this issue.

3 participants