Skip to content

Conversation

pnkvalavala
Copy link
Contributor

@pnkvalavala pnkvalavala commented Aug 2, 2025

Removes redundant schema information from messages when using JSON_SCHEMA mode.

Why This Change?

JSON mode (response_format: {"type": "json_object"}) - OpenAI docs require explicit JSON instruction in messages since no schema is provided in response_format.
https://platform.openai.com/docs/guides/structured-outputs?api-mode=chat#json-mode

When using JSON mode, you must always instruct the model to produce JSON via some message in the conversation, for example via your system message. If you don't include an explicit instruction to generate JSON, the model may generate an unending stream of whitespace and the request may run continually until it reaches the token limit. To help ensure you don't forget, the API will throw an error if the string "JSON" does not appear somewhere in the context.

JSON_SCHEMA mode (response_format: {"type": "json_schema", ...}) - Schema is already provided in response_format. Adding the same schema to messages creates redundancy, increases token consumption unnecessarily, and provides no additional value to the model.

Changes

  • JSON_SCHEMA mode: No schema added to messages (schema already in response_format)
  • JSON and MD_JSON modes: Unchanged behavior (still add schema to messages as required)

Important

Removes redundant schema from messages in JSON_SCHEMA mode in handle_json_modes() in utils.py, reducing token consumption.

  • Behavior:
    • In handle_json_modes() in utils.py, JSON_SCHEMA mode no longer adds schema to messages, as it's already in response_format.
    • JSON and MD_JSON modes remain unchanged, still adding schema to messages.
  • Rationale:
    • Reduces redundancy and token consumption in JSON_SCHEMA mode by not duplicating schema in messages.

This description was created by Ellipsis for f3af7fb. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to f3af7fb in 1 minute and 14 seconds. Click for details.
  • Reviewed 44 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. instructor/providers/openai/utils.py:436
  • Draft comment:
    Good change: The 'if mode != Mode.JSON_SCHEMA' condition properly avoids adding duplicate schema information in JSON_SCHEMA mode. Consider adding an inline comment to clarify this design decision for future maintainers.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 85% The comment is about documenting a design decision in the code. While the suggestion is reasonable, the function name handle_json_modes and the context make it fairly clear what's happening. The code is not complex enough to warrant additional inline documentation. The comment also starts with "Good change:" which is not actionable. The design decision could be non-obvious to new contributors. Documentation can help prevent future regressions. The function name and surrounding context provide sufficient clarity. The code change is straightforward and the reason for skipping JSON_SCHEMA mode is evident from the mode handling logic throughout the file. Delete the comment as it suggests adding documentation that isn't necessary given the clear context and straightforward code change.
2. instructor/providers/openai/utils.py:437
  • Draft comment:
    Note: The code assumes new_kwargs['messages'] is non-empty. It might be prudent to add a guard or document this requirement to avoid potential IndexError.
  • Reason this comment was not posted:
    Confidence changes required: 80% <= threshold 85% None

Workflow ID: wflow_bjqLYxK1hjtcwoBW

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@pnkvalavala
Copy link
Contributor Author

Hi @jxnl, just following up on this PR. It’s a small change that removes redundant schema duplication in JSON_SCHEMA mode, reducing token usage without affecting JSON or MD_JSON modes.

Would appreciate it if you could take a look when you get a chance.

@jxnl jxnl enabled auto-merge (squash) August 20, 2025 18:16
@pnkvalavala
Copy link
Contributor Author

Hi @jxnl, quick follow-up. I see auto-merge is enabled, but CI is blocking; logs show OIDC token errors in the claude-review job and missing provider API keys in provider tests. Seems unrelated to this diff, but happy to tweak if needed.

@jxnl jxnl disabled auto-merge September 11, 2025 18:30
@jxnl jxnl merged commit 79e913d into 567-labs:main Sep 11, 2025
3 of 11 checks passed
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