-
Notifications
You must be signed in to change notification settings - Fork 855
Handle GPT-5 token params by provider #922
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,21 @@ | ||||||
| const GPT5_CHAT_COMPLETIONS_MODEL_PATTERN = /(^|\/)gpt-5([.-]|$)/ | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OpenAI's
Suggested change
|
||||||
|
|
||||||
| function shouldUseMaxCompletionTokens(provider, model) { | ||||||
| const normalizedProvider = String(provider || '').toLowerCase() | ||||||
| const normalizedModel = String(model || '').toLowerCase() | ||||||
|
|
||||||
| switch (true) { | ||||||
| case normalizedProvider === 'openai' && | ||||||
| GPT5_CHAT_COMPLETIONS_MODEL_PATTERN.test(normalizedModel): | ||||||
| return true | ||||||
| default: | ||||||
| return false | ||||||
| } | ||||||
|
Comment on lines
+7
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
| } | ||||||
|
|
||||||
| export function getChatCompletionsTokenParams(provider, model, maxResponseTokenLength) { | ||||||
| if (shouldUseMaxCompletionTokens(provider, model)) | ||||||
| return { max_completion_tokens: maxResponseTokenLength } | ||||||
|
|
||||||
| return { max_tokens: maxResponseTokenLength } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import test from 'node:test' | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file was placed at the wrong place 😓 |
||
| import assert from 'node:assert/strict' | ||
| import { getChatCompletionsTokenParams } from './openai-token-params.mjs' | ||
|
PeterDaveHello marked this conversation as resolved.
|
||
|
|
||
|
Comment on lines
+1
to
+4
|
||
| test('uses max_completion_tokens for gpt-5.x chat models', () => { | ||
| assert.deepEqual(getChatCompletionsTokenParams('openai', 'gpt-5.2-chat-latest', 1024), { | ||
| max_completion_tokens: 1024, | ||
| }) | ||
| }) | ||
|
|
||
| test('uses max_completion_tokens for provider-prefixed gpt-5.x models', () => { | ||
| assert.deepEqual(getChatCompletionsTokenParams('openai', 'openai/gpt-5.2', 2048), { | ||
| max_completion_tokens: 2048, | ||
| }) | ||
| }) | ||
|
|
||
| test('uses max_completion_tokens for gpt-5 baseline model name', () => { | ||
| assert.deepEqual(getChatCompletionsTokenParams('openai', 'gpt-5', 1536), { | ||
| max_completion_tokens: 1536, | ||
| }) | ||
| }) | ||
|
|
||
| test('uses max_tokens for non gpt-5 chat models', () => { | ||
| assert.deepEqual(getChatCompletionsTokenParams('openai', 'gpt-4o', 512), { | ||
| max_tokens: 512, | ||
| }) | ||
| }) | ||
|
|
||
| test('uses max_tokens for lookalike model names', () => { | ||
| assert.deepEqual(getChatCompletionsTokenParams('openai', 'my-gpt-5-clone', 640), { | ||
| max_tokens: 640, | ||
| }) | ||
| }) | ||
|
|
||
| test('uses max_tokens for empty model values', () => { | ||
| assert.deepEqual(getChatCompletionsTokenParams('openai', '', 256), { | ||
| max_tokens: 256, | ||
| }) | ||
| }) | ||
|
|
||
| test('uses max_tokens for non OpenAI providers even with gpt-5 models', () => { | ||
| assert.deepEqual(getChatCompletionsTokenParams('some-proxy-provider', 'openai/gpt-5.2', 257), { | ||
| max_tokens: 257, | ||
| }) | ||
| }) | ||
|
|
||
| test('uses max_completion_tokens for mixed-case OpenAI provider and model', () => { | ||
| assert.deepEqual(getChatCompletionsTokenParams('OpenAI', 'GPT-5.1', 258), { | ||
| max_completion_tokens: 258, | ||
| }) | ||
| }) | ||
|
|
||
| test('uses max_tokens when provider is undefined', () => { | ||
| assert.deepEqual(getChatCompletionsTokenParams(undefined, 'gpt-5.1', 259), { | ||
| max_tokens: 259, | ||
| }) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generateAnswersWithCustomApialways passes'custom'as the provider, which guaranteesmax_tokenseven when the custom URL/model are actually targeting OpenAI GPT‑5 chat completions (a configuration that currently exists viacustomModelApiUrl/customModelName). That setup would still hit the original "Unsupported parameter: 'max_tokens'" error. Consider accepting aproviderargument (fromsession.apiMode/ config) or inferring it fromapiUrlso GPT‑5+OpenAI can usemax_completion_tokenswhen appropriate.