Skip to content

update gpt-5 models#57

Merged
j4ys0n merged 1 commit into
mainfrom
ptc
Mar 17, 2026
Merged

update gpt-5 models#57
j4ys0n merged 1 commit into
mainfrom
ptc

Conversation

@j4ys0n
Copy link
Copy Markdown
Contributor

@j4ys0n j4ys0n commented Mar 17, 2026

No description provided.

Copilot AI review requested due to automatic review settings March 17, 2026 12:59
@j4ys0n
Copy link
Copy Markdown
Contributor Author

j4ys0n commented Mar 17, 2026

Automated review 🤖

Summary of Changes
This PR adds comprehensive support for OpenAI’s GPT-5 family of models in the SDK, including model-specific configuration for reasoning effort, verbosity, and sampling behavior. Version is bumped to 1.12.0. A new gpt5.support.ts file defines per-model capabilities (e.g., chatCompletionsSupported, allowedReasoningEfforts, supportsSampling), and the OpenAI mapper enforces these rules—dropping unsupported parameters (e.g., temperature, top_p) or throwing UnsupportedFeatureError for responses-only models like gpt-5-pro. New reasoningEffort and verbosity fields are added to GenerateParams, and unit tests cover key GPT-5 behaviors.

Key Changes & Positives

  • ✅ Introduces robust, extensible model capability table (gpt5.support.ts) with exact per-model behavior definitions (e.g., gpt-5.2 supports xhigh, gpt-5-chat-latest allows sampling) 🟢
  • ✅ Adds safe fallback for unknown future GPT-5 models: preserves max_tokens, drops all GPT-5-specific fields 🟢
  • ✅ Enforces model constraints via UnsupportedFeatureError for non-chat-completions models (e.g., gpt-5-pro) 🟢
  • ✅ Adds reasoningEffort and verbosity to public GenerateParams interface with clear JSDoc 🟢

Potential Issues & Recommendations

  1. Issue / Risk: The getGpt5Support function uses model.startsWith(${key}-) for prefix matching, which may incorrectly match unrelated models (e.g., gpt-5.2-foo matches gpt-5.2, but gpt-5.20 would also match gpt-5.2).
    Impact: Could apply incorrect GPT-5.2 rules to future models like gpt-5.20 if released.
    Recommendation: Use regex ^${key}(-|$) or exact match first, then prefix match only if no exact match exists.
    Status: 🟡 Needs review

  2. Issue / Risk: DEBUG log in rosetta-ai.ts only covers Anthropic streams; GPT-5 OpenAI mappings lack similar debug logging despite complex parameter transformations.
    Impact: Harder to diagnose GPT-5-specific mapping failures in production.
    Recommendation: Extend debug logging to OpenAI GPT-5 paths (e.g., log support object and effective reasoning_effort, verbosity, sampling decision).
    Status: 🟡 Needs review

Language/Framework Checks

  • TypeScript types correctly define ReasoningEffort ('none' | 'minimal' | 'low' | 'medium' | 'high' | 'xhigh') and Verbosity ('low' | 'medium' | 'high') in params.types.ts
  • gpt5.support.ts imports ReasoningEffort from ../../types, but types/index.ts exports GenerateParamsProviderState—ensure types.ts re-exports ReasoningEffort (not visible in diff, but critical for build)
  • getGpt5Support sorts keys by descending length (b.length - a.length) to prioritize longer prefixes (e.g., gpt-5.2 over gpt-5)—correct and necessary

Security & Privacy

  • No sensitive data exposure or hardcoded secrets detected.
  • Sampling parameter suppression for non-allowed models (temperature, top_p) prevents accidental leakage of user-controlled randomness into models where it’s disallowed (e.g., gpt-5, gpt-5.4).

Build/CI & Ops

  • No changes to build scripts or dependencies—safe for deployment.
  • New gpt5.support.ts file adds ~160 lines but is self-contained; no impact on bundle size beyond inclusion in dist.

Tests

  • Unit tests added for core GPT-5 behaviors: default effort, sampling suppression, verbosity, prefix matching, fixed effort, and error cases.
  • Missing test: verify verbosity is dropped for models without supportsVerbosity (e.g., gpt-5-codex)—add test case.
  • Missing test: ensure maxTokens is preserved for unknown models but dropped for known ones (e.g., gpt-5.4-pro should drop maxTokens).

Approval Recommendation
Approve with caveats

  • Address prefix-matching edge case in getGpt5Support (regex or stricter matching)
  • Add missing verbosity/maxTokens test cases for unsupported models
  • Confirm ReasoningEffort is exported from types/index.ts (verify in build)

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the OpenAI Chat Completions parameter mapping to be GPT‑5-family aware, introducing explicit reasoning/verbosity controls and model-specific capability rules.

Changes:

  • Add reasoningEffort and verbosity to GenerateParams and plumb them through base param mapping.
  • Implement a centralized GPT‑5 support table (getGpt5Support) and update OpenAIMapper to enforce/normalize GPT‑5 Chat Completions behavior (sampling, reasoning, verbosity, unsupported variants).
  • Expand unit tests to cover GPT‑5 / GPT‑5.2 / dated variants / unsupported variants / unknown future variants.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/unit/core/mapping/openai.mapper.spec.ts Adds test coverage for GPT‑5-family normalization and rejection behavior in Chat Completions mapping.
src/types/params.types.ts Introduces ReasoningEffort / Verbosity types and exposes them on GenerateParams.
src/core/rosetta-ai.ts Adds DEBUG-only logging of mapped Anthropic stream requests.
src/core/mapping/openai.mapper.ts Implements GPT‑5 capability-aware mapping (reasoning, verbosity, sampling, unsupported model rejection).
src/core/mapping/gpt5.support.ts New GPT‑5 capability table + exact/prefix lookup helper used by the OpenAI mapper.
src/core/mapping/common.utils.ts Extends mapBaseParams to carry reasoningEffort and verbosity.
package.json Bumps package version to 1.12.0.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread src/core/rosetta-ai.ts
const mappedParams = mapper.mapToProviderParams ? mapper.mapToProviderParams(effectiveParams) : effectiveParams

if (process.env.DEBUG === 'true' && providerKey === Provider.Anthropic) {
console.log(`[ROSETTA DEBUG] Mapped Anthropic stream request:\n${JSON.stringify(mappedParams, null, 2)}`)
expect(result.top_p).toBe(0.9)
})

it('[Medium] should drop sampling and default reasoning for gpt-5', () => {
Comment thread src/core/rosetta-ai.ts
Comment on lines +730 to +732
if (process.env.DEBUG === 'true' && providerKey === Provider.Anthropic) {
console.log(`[ROSETTA DEBUG] Mapped Anthropic stream request:\n${JSON.stringify(mappedParams, null, 2)}`)
}
Comment thread src/core/rosetta-ai.ts
const mappedParams = mapper.mapToProviderParams ? mapper.mapToProviderParams(effectiveParams) : effectiveParams

if (process.env.DEBUG === 'true' && providerKey === Provider.Anthropic) {
console.log(`[ROSETTA DEBUG] Mapped Anthropic stream request:\n${JSON.stringify(mappedParams, null, 2)}`)
@j4ys0n j4ys0n merged commit 039f0c1 into main Mar 17, 2026
5 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