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

Append JSON instructions to OpenAI system prompt #615

Merged

Conversation

navidemad
Copy link
Contributor

@navidemad navidemad commented Jan 25, 2025

In order to fix response_format json_object introduced in the following commit: a0e2fef

INFO -- : response: {
  "error": {
    "message": "'messages' must contain the word 'json' in some form, to use 'response_format' of type 'json_object'.",
    "type": "invalid_request_error",
    "param": "messages",
    "code": null
  }
}

@davidwessman
Copy link
Collaborator

@navidemad can you fix the linting error? (Add an ignore for the too long line).

@glebm do you have any input on how we should handle these instructions? I have not used OpenAI for any translation.

@navidemad
Copy link
Contributor Author

@davidwessman I fixed the linting error as requested

@glebm
Copy link
Owner

glebm commented Feb 9, 2025

I don't do ruby at all anymore but this PR makes sense to me in principle.

@davidwessman
Copy link
Collaborator

@jaredmoody @dankimio @yshmarov you have recently done Open AI-issues on the repo, does this change look good to you as well? 🙂

@dankimio
Copy link

@jaredmoody yes, this looks good. not sure if there's a need for a separate JSON_FORMAT_INSTRUCTIONS_SYSTEM_PROMPT constant. wouldn't it be easier to include that phrase in the DEFAULT_SYSTEM_PROMPT? but nice approach with concat and already better than what we have currently.

@navidemad
Copy link
Contributor Author

@dankimio I made it that way because if someone sets the :openai_system_prompt option—for example, to create a custom prompt—but forgets to include this JSON part, it will also fail. This way, it will be appended automatically, preventing this issue.

@davidwessman davidwessman merged commit c560714 into glebm:main Feb 10, 2025
7 of 9 checks passed
@jaredmoody
Copy link
Contributor

We do something very similar to this in our prompt, so LGTM.

As a future enhancement though, it would be nice to request JSON from the API by passing response_format: { type: "json_object" } so that the result is always JSON and it's not necessary to include that in the prompt.

https://github.com/alexrudall/ruby-openai?tab=readme-ov-file#json-mode

@navidemad
Copy link
Contributor Author

We do something very similar to this in our prompt, so LGTM.

As a future enhancement though, it would be nice to request JSON from the API by passing response_format: { type: "json_object" } so that the result is always JSON and it's not necessary to include that in the prompt.

https://github.com/alexrudall/ruby-openai?tab=readme-ov-file#json-mode

https://github.com/glebm/i18n-tasks/blob/main/lib/i18n/tasks/translators/openai_translator.rb#L101
It is already.

@jaredmoody
Copy link
Contributor

Ah, great, I missed when that was added last month.

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.

5 participants