Skip to content

feat: add GPT-5.1 model support#37

Merged
Kamilbenkirane merged 2 commits intomainfrom
feat/add-gpt-5.1-model
Nov 16, 2025
Merged

feat: add GPT-5.1 model support#37
Kamilbenkirane merged 2 commits intomainfrom
feat/add-gpt-5.1-model

Conversation

@Kamilbenkirane
Copy link
Copy Markdown
Member

@Kamilbenkirane Kamilbenkirane commented Nov 16, 2025

Add GPT-5.1 and GPT-5.1 Codex models to OpenAI text-generation provider.

Changes

  • Added GPT-5.1 model with:

    • Max tokens: 128,000
    • Thinking budget: minimal, low, medium, high
    • Verbosity: low, medium, high
    • Output schema support
    • Streaming support
  • Added GPT-5.1 Codex model with:

    • Max tokens: 128,000
    • Thinking budget: minimal, low, medium, high
    • Verbosity: low, medium, high
    • Output schema support
    • Streaming support

GPT-5.1 is OpenAI's flagship model for coding and agentic tasks with configurable reasoning effort.

@Kamilbenkirane Kamilbenkirane merged commit db8cfdd into main Nov 16, 2025
11 checks passed
@claude
Copy link
Copy Markdown

claude Bot commented Nov 16, 2025

Pull Request Review: Add GPT-5.1 Model Support

Overview

This PR adds support for OpenAI's GPT-5.1 model to the text-generation provider. The implementation is clean and follows the existing patterns in the codebase.

Code Quality ✅

Strengths:

  • Consistent with existing patterns: The model definition follows the same structure as other GPT-5 family models (gpt-5, gpt-5-mini, gpt-5-nano)
  • Proper parameter constraints: Correctly defines max_tokens, thinking_budget, verbosity, and output_schema parameters
  • Well-positioned: Model is added in a logical location (between gpt-5 and gpt-5-mini)
  • Clean implementation: No unnecessary changes, focused scope

Analysis by Category

1. Best Practices

  • Follows the established model registration pattern seen in packages/text-generation/src/celeste_text_generation/providers/openai/models.py:8-127
  • Uses appropriate constraint types (Range, Choice, Schema)
  • Maintains consistent code formatting and structure

2. Potential Issues ⚠️

Minor Concerns:

  1. Model Verification:

    • OpenAI has not publicly announced GPT-5.1 as of January 2025. Is this a hypothetical model for testing, or based on internal/beta access?
    • Recommendation: Add a comment or documentation clarifying the model's status if it's not yet publicly available
  2. Verbosity Parameter Uniqueness:

    • GPT-5.1 is the only model in the OpenAI provider with the VERBOSITY parameter
    • GPT-5, GPT-5-mini, and GPT-5-nano don't have this parameter
    • Question: Is this intentional? Should other GPT-5 family models also support verbosity?
    • The VerbosityMapper exists at packages/text-generation/src/celeste_text_generation/providers/openai/parameters.py:206-223, suggesting it's a supported feature
  3. Parameter Consistency:

    • Older models (gpt-4o, gpt-4-turbo, etc.) include TEMPERATURE parameter, but GPT-5 family models don't
    • This appears intentional based on the pattern, but worth confirming this matches OpenAI's API

3. Performance Considerations

  • No performance impact - this is purely declarative configuration
  • Model definition is loaded at startup and cached in the registry
  • The parameter mappers are already implemented and tested

4. Security Concerns

  • No security issues identified
  • All parameters use proper constraint validation
  • Schema() constraint properly handles output schema validation
  • No injection or validation bypass risks

5. Test Coverage ⚠️

Observations:

  • No tests were added for the new model
  • Existing integration tests at packages/text-generation/tests/integration_tests/test_text_generation/test_generate.py:8-16 don't include GPT-5.1
  • Unit tests for model registry exist at tests/unit_tests/test_models.py but don't specifically test GPT-5.1

Recommendations:

  1. Consider adding an integration test for GPT-5.1 with verbosity parameter (if the model is available for testing)
  2. At minimum, verify the model registration doesn't break existing tests

Specific Code Review

Location: packages/text-generation/src/celeste_text_generation/providers/openai/models.py:74-89

Model(
    id="gpt-5.1",
    provider=Provider.OPENAI,
    display_name="GPT-5.1",
    streaming=True,
    parameter_constraints={
        Parameter.MAX_TOKENS: Range(min=1, max=128000),
        TextGenerationParameter.THINKING_BUDGET: Choice(
            options=["minimal", "low", "medium", "high"]
        ),
        TextGenerationParameter.VERBOSITY: Choice(
            options=["low", "medium", "high"]
        ),
        TextGenerationParameter.OUTPUT_SCHEMA: Schema(),
    },
),

Positive aspects:

  • ✅ Correct use of Choice constraint for discrete options
  • ✅ Max tokens set to 128,000 (consistent with GPT-5 family)
  • ✅ Streaming support enabled
  • ✅ All required fields present

Questions for Author

  1. Model Availability: Is GPT-5.1 currently available via OpenAI's API, or is this being added preemptively?
  2. Verbosity Scope: Should the VERBOSITY parameter also be added to gpt-5, gpt-5-mini, and gpt-5-nano models for consistency?
  3. Documentation: Are there any OpenAI docs or specifications you're working from that we should reference?

Recommendations

Required: None ✅

The code is functional and follows best practices.

Suggested Improvements:

  1. Documentation: Add a comment about model availability status if it's not yet publicly released
  2. Consistency: Consider whether VERBOSITY should be added to other GPT-5 family models
  3. Testing: Add basic integration test when the model becomes available (can be in a follow-up PR)

Summary

Verdict: ✅ Approve with minor suggestions

This is a well-implemented addition that follows the existing codebase patterns. The code quality is good, and there are no blocking issues. The main concerns are around model availability/documentation and parameter consistency across the GPT-5 family, which can be addressed with clarification or follow-up changes.

The implementation correctly:

  • Defines the model with appropriate constraints
  • Uses existing parameter mappers (ThinkingBudgetMapper, VerbosityMapper, etc.)
  • Maintains backward compatibility
  • Follows the established model registration pattern

Great job maintaining consistency with the existing codebase! 🎉

@claude
Copy link
Copy Markdown

claude Bot commented Nov 16, 2025

Code Review for PR #37: feat: add GPT-5.1 model support

Thank you for this contribution! I've reviewed the changes and have the following feedback:

Code Quality and Best Practices

Positive:

  • The code follows the existing patterns and conventions established in the codebase
  • Model definitions are consistent with other GPT models in the file
  • Proper use of parameter constraints with type-safe enums and constraints

Concerns:

  1. Model Verification Needed: The PR adds support for GPT-5.1 and GPT-5.1 Codex models, but these don't appear to be officially announced OpenAI models. Can you provide:

    • Official OpenAI documentation or API reference for these models?
    • Confirmation that these model IDs are correct and available in the OpenAI API?
  2. Duplicate Code: Both gpt-5.1 and gpt-5.1-codex have identical parameter configurations. Consider whether these models truly have identical constraints or if there are differences to account for.

Potential Bugs or Issues

  1. Missing TEMPERATURE Parameter: Unlike other GPT models (gpt-4o, gpt-4-turbo, gpt-3.5-turbo, gpt-4.1), the new GPT-5.1 models don't include Parameter.TEMPERATURE in their constraints. Is this intentional? Looking at gpt-5 (line 62-73), it also lacks temperature, but this should be verified against OpenAI's actual API specification.

  2. Verbosity Parameter Validation: The VERBOSITY parameter is added here but not present in the base gpt-5 model (line 62-73). This inconsistency should be clarified:

    • Does GPT-5.1 support verbosity while GPT-5 doesn't?
    • Or should GPT-5 also be updated to include verbosity?

Performance Considerations

  • No performance issues identified. The static model definitions have minimal runtime impact.

Security Concerns

  • No security issues identified. The constraints properly validate input parameters.

Test Coverage

Missing Tests: This PR lacks test coverage. I recommend adding:

  1. Unit tests to verify the model configurations are registered correctly
  2. Integration tests to confirm the models work with the OpenAI provider (if you have API access)

Example test to add in packages/text-generation/tests/unit_tests/providers/openai/:

def test_gpt_5_1_models_registered():
    """Verify GPT-5.1 models are properly configured."""
    from celeste import Provider, get_model
    from celeste_text_generation.parameters import TextGenerationParameter
    from celeste.core import Parameter
    
    gpt_5_1 = get_model("gpt-5.1", Provider.OPENAI)
    assert gpt_5_1 is not None
    assert gpt_5_1.display_name == "GPT-5.1"
    assert gpt_5_1.streaming is True
    assert Parameter.MAX_TOKENS in gpt_5_1.supported_parameters
    assert TextGenerationParameter.THINKING_BUDGET in gpt_5_1.supported_parameters
    assert TextGenerationParameter.VERBOSITY in gpt_5_1.supported_parameters
    assert TextGenerationParameter.OUTPUT_SCHEMA in gpt_5_1.supported_parameters

Recommendations

  1. Verify Model Existence: Confirm these models are officially available in OpenAI's API
  2. Add Documentation: Update the PR description or relevant docs to include:
    • When these models were released
    • Link to OpenAI documentation
    • Use cases for GPT-5.1 vs GPT-5.1 Codex
  3. Consistency Review: Review whether gpt-5 should also have the VERBOSITY parameter
  4. Add Tests: Include unit tests for the new models
  5. Consider Temperature: Verify whether temperature control should be available for these models

Would you be able to address these points? Happy to help if you need any clarification!

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.

1 participant