diff --git a/.changeset/vast-weeks-fetch.md b/.changeset/vast-weeks-fetch.md new file mode 100644 index 00000000..eb8e72e3 --- /dev/null +++ b/.changeset/vast-weeks-fetch.md @@ -0,0 +1,7 @@ +--- +"task-master-ai": minor +--- + +Add GPT-5 support with proper parameter handling + +- Added GPT-5 model to supported models configuration with SWE score of 0.749 diff --git a/CLAUDE.md b/CLAUDE.md index ec26a18a..57471525 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -3,3 +3,7 @@ ## Task Master AI Instructions **Import Task Master's development workflow commands and guidelines, treat as if import is in the main CLAUDE.md file.** @./.taskmaster/CLAUDE.md + +## Changeset Guidelines + +- When creating changesets, remember that it's user-facing, meaning we don't have to get into the specifics of the code, but rather mention what the end-user is getting or fixing from this changeset. \ No newline at end of file diff --git a/scripts/modules/config-manager.js b/scripts/modules/config-manager.js index f71a0662..81da1488 100644 --- a/scripts/modules/config-manager.js +++ b/scripts/modules/config-manager.js @@ -557,6 +557,7 @@ function getParametersForRole(role, explicitRoot = null) { const providerName = roleConfig.provider; let effectiveMaxTokens = roleMaxTokens; // Start with the role's default + let effectiveTemperature = roleTemperature; // Start with the role's default try { // Find the model definition in MODEL_MAP @@ -583,6 +584,20 @@ function getParametersForRole(role, explicitRoot = null) { `No valid model-specific max_tokens override found for ${modelId}. Using role default: ${roleMaxTokens}` ); } + + // Check if a model-specific temperature is defined + if ( + modelDefinition && + typeof modelDefinition.temperature === 'number' && + modelDefinition.temperature >= 0 && + modelDefinition.temperature <= 1 + ) { + effectiveTemperature = modelDefinition.temperature; + log( + 'debug', + `Applying model-specific temperature (${modelDefinition.temperature}) for ${modelId}` + ); + } } else { // Special handling for custom OpenRouter models if (providerName === CUSTOM_PROVIDERS.OPENROUTER) { @@ -603,15 +618,16 @@ function getParametersForRole(role, explicitRoot = null) { } catch (lookupError) { log( 'warn', - `Error looking up model-specific max_tokens for ${modelId}: ${lookupError.message}. Using role default: ${roleMaxTokens}` + `Error looking up model-specific parameters for ${modelId}: ${lookupError.message}. Using role defaults.` ); - // Fallback to role default on error + // Fallback to role defaults on error effectiveMaxTokens = roleMaxTokens; + effectiveTemperature = roleTemperature; } return { maxTokens: effectiveMaxTokens, - temperature: roleTemperature + temperature: effectiveTemperature }; } diff --git a/scripts/modules/supported-models.json b/scripts/modules/supported-models.json index 0409cbc8..f8bef43c 100644 --- a/scripts/modules/supported-models.json +++ b/scripts/modules/supported-models.json @@ -239,6 +239,18 @@ }, "allowed_roles": ["research"], "supported": true + }, + { + "id": "gpt-5", + "swe_score": 0.749, + "cost_per_1m_tokens": { + "input": 5.0, + "output": 20.0 + }, + "allowed_roles": ["main", "fallback"], + "max_tokens": 100000, + "temperature": 1, + "supported": true } ], "google": [ diff --git a/src/ai-providers/base-provider.js b/src/ai-providers/base-provider.js index 90e18a1e..441b6ff7 100644 --- a/src/ai-providers/base-provider.js +++ b/src/ai-providers/base-provider.js @@ -61,8 +61,11 @@ export class BaseAIProvider { ) { throw new Error('Temperature must be between 0 and 1'); } - if (params.maxTokens !== undefined && params.maxTokens <= 0) { - throw new Error('maxTokens must be greater than 0'); + if (params.maxTokens !== undefined) { + const maxTokens = Number(params.maxTokens); + if (!Number.isFinite(maxTokens) || maxTokens <= 0) { + throw new Error('maxTokens must be a finite number greater than 0'); + } } } @@ -122,6 +125,37 @@ export class BaseAIProvider { throw new Error('getRequiredApiKeyName must be implemented by provider'); } + /** + * Determines if a model requires max_completion_tokens instead of maxTokens + * Can be overridden by providers to specify their model requirements + * @param {string} modelId - The model ID to check + * @returns {boolean} True if the model requires max_completion_tokens + */ + requiresMaxCompletionTokens(modelId) { + return false; // Default behavior - most models use maxTokens + } + + /** + * Prepares token limit parameter based on model requirements + * @param {string} modelId - The model ID + * @param {number} maxTokens - The maximum tokens value + * @returns {object} Object with either maxTokens or max_completion_tokens + */ + prepareTokenParam(modelId, maxTokens) { + if (maxTokens === undefined) { + return {}; + } + + // Ensure maxTokens is an integer + const tokenValue = Math.floor(Number(maxTokens)); + + if (this.requiresMaxCompletionTokens(modelId)) { + return { max_completion_tokens: tokenValue }; + } else { + return { maxTokens: tokenValue }; + } + } + /** * Generates text using the provider's model */ @@ -139,7 +173,7 @@ export class BaseAIProvider { const result = await generateText({ model: client(params.modelId), messages: params.messages, - maxTokens: params.maxTokens, + ...this.prepareTokenParam(params.modelId, params.maxTokens), temperature: params.temperature }); @@ -175,7 +209,7 @@ export class BaseAIProvider { const stream = await streamText({ model: client(params.modelId), messages: params.messages, - maxTokens: params.maxTokens, + ...this.prepareTokenParam(params.modelId, params.maxTokens), temperature: params.temperature }); @@ -216,7 +250,7 @@ export class BaseAIProvider { messages: params.messages, schema: zodSchema(params.schema), mode: params.mode || 'auto', - maxTokens: params.maxTokens, + ...this.prepareTokenParam(params.modelId, params.maxTokens), temperature: params.temperature }); diff --git a/src/ai-providers/openai.js b/src/ai-providers/openai.js index 4f7b67eb..adf55822 100644 --- a/src/ai-providers/openai.js +++ b/src/ai-providers/openai.js @@ -20,6 +20,16 @@ export class OpenAIProvider extends BaseAIProvider { return 'OPENAI_API_KEY'; } + /** + * Determines if a model requires max_completion_tokens instead of maxTokens + * GPT-5 models require max_completion_tokens parameter + * @param {string} modelId - The model ID to check + * @returns {boolean} True if the model requires max_completion_tokens + */ + requiresMaxCompletionTokens(modelId) { + return modelId && modelId.startsWith('gpt-5'); + } + /** * Creates and returns an OpenAI client instance. * @param {object} params - Parameters for client initialization diff --git a/tests/unit/ai-providers/openai.test.js b/tests/unit/ai-providers/openai.test.js new file mode 100644 index 00000000..9c090a59 --- /dev/null +++ b/tests/unit/ai-providers/openai.test.js @@ -0,0 +1,238 @@ +/** + * Tests for OpenAI Provider - Token parameter handling for GPT-5 + * + * This test suite covers: + * 1. Correct identification of GPT-5 models requiring max_completion_tokens + * 2. Token parameter preparation for different model types + * 3. Validation of maxTokens parameter + * 4. Integer coercion of token values + */ + +import { jest } from '@jest/globals'; + +// Mock the utils module to prevent logging during tests +jest.mock('../../../scripts/modules/utils.js', () => ({ + log: jest.fn() +})); + +// Import the provider +import { OpenAIProvider } from '../../../src/ai-providers/openai.js'; + +describe('OpenAIProvider', () => { + let provider; + + beforeEach(() => { + provider = new OpenAIProvider(); + jest.clearAllMocks(); + }); + + describe('requiresMaxCompletionTokens', () => { + it('should return true for GPT-5 models', () => { + expect(provider.requiresMaxCompletionTokens('gpt-5')).toBe(true); + expect(provider.requiresMaxCompletionTokens('gpt-5-mini')).toBe(true); + expect(provider.requiresMaxCompletionTokens('gpt-5-nano')).toBe(true); + expect(provider.requiresMaxCompletionTokens('gpt-5-turbo')).toBe(true); + }); + + it('should return false for non-GPT-5 models', () => { + expect(provider.requiresMaxCompletionTokens('gpt-4')).toBe(false); + expect(provider.requiresMaxCompletionTokens('gpt-4o')).toBe(false); + expect(provider.requiresMaxCompletionTokens('gpt-3.5-turbo')).toBe(false); + expect(provider.requiresMaxCompletionTokens('o1')).toBe(false); + expect(provider.requiresMaxCompletionTokens('o1-mini')).toBe(false); + }); + + it('should handle null/undefined modelId', () => { + expect(provider.requiresMaxCompletionTokens(null)).toBeFalsy(); + expect(provider.requiresMaxCompletionTokens(undefined)).toBeFalsy(); + expect(provider.requiresMaxCompletionTokens('')).toBeFalsy(); + }); + }); + + describe('prepareTokenParam', () => { + it('should return max_completion_tokens for GPT-5 models', () => { + const result = provider.prepareTokenParam('gpt-5', 1000); + expect(result).toEqual({ max_completion_tokens: 1000 }); + }); + + it('should return maxTokens for non-GPT-5 models', () => { + const result = provider.prepareTokenParam('gpt-4', 1000); + expect(result).toEqual({ maxTokens: 1000 }); + }); + + it('should coerce token value to integer', () => { + // Float values + const result1 = provider.prepareTokenParam('gpt-5', 1000.7); + expect(result1).toEqual({ max_completion_tokens: 1000 }); + + const result2 = provider.prepareTokenParam('gpt-4', 1000.7); + expect(result2).toEqual({ maxTokens: 1000 }); + + // String float + const result3 = provider.prepareTokenParam('gpt-5', '1000.7'); + expect(result3).toEqual({ max_completion_tokens: 1000 }); + + // String integers (common CLI input path) + expect(provider.prepareTokenParam('gpt-5', '1000')).toEqual({ + max_completion_tokens: 1000 + }); + expect(provider.prepareTokenParam('gpt-4', '1000')).toEqual({ + maxTokens: 1000 + }); + }); + + it('should return empty object for undefined maxTokens', () => { + const result = provider.prepareTokenParam('gpt-5', undefined); + expect(result).toEqual({}); + }); + + it('should handle edge cases', () => { + // Test with 0 (should still pass through as 0) + const result1 = provider.prepareTokenParam('gpt-5', 0); + expect(result1).toEqual({ max_completion_tokens: 0 }); + + // Test with string number + const result2 = provider.prepareTokenParam('gpt-5', '100'); + expect(result2).toEqual({ max_completion_tokens: 100 }); + + // Test with negative number (will be floored, validation happens elsewhere) + const result3 = provider.prepareTokenParam('gpt-4', -10.5); + expect(result3).toEqual({ maxTokens: -11 }); + }); + }); + + describe('validateOptionalParams', () => { + it('should accept valid maxTokens values', () => { + expect(() => + provider.validateOptionalParams({ maxTokens: 1000 }) + ).not.toThrow(); + expect(() => + provider.validateOptionalParams({ maxTokens: 1 }) + ).not.toThrow(); + expect(() => + provider.validateOptionalParams({ maxTokens: '1000' }) + ).not.toThrow(); + }); + + it('should reject invalid maxTokens values', () => { + expect(() => provider.validateOptionalParams({ maxTokens: 0 })).toThrow( + Error + ); + expect(() => provider.validateOptionalParams({ maxTokens: -1 })).toThrow( + Error + ); + expect(() => provider.validateOptionalParams({ maxTokens: NaN })).toThrow( + Error + ); + expect(() => + provider.validateOptionalParams({ maxTokens: Infinity }) + ).toThrow(Error); + expect(() => + provider.validateOptionalParams({ maxTokens: 'invalid' }) + ).toThrow(Error); + }); + + it('should accept valid temperature values', () => { + expect(() => + provider.validateOptionalParams({ temperature: 0 }) + ).not.toThrow(); + expect(() => + provider.validateOptionalParams({ temperature: 0.5 }) + ).not.toThrow(); + expect(() => + provider.validateOptionalParams({ temperature: 1 }) + ).not.toThrow(); + }); + + it('should reject invalid temperature values', () => { + expect(() => + provider.validateOptionalParams({ temperature: -0.1 }) + ).toThrow(Error); + expect(() => + provider.validateOptionalParams({ temperature: 1.1 }) + ).toThrow(Error); + }); + }); + + describe('getRequiredApiKeyName', () => { + it('should return OPENAI_API_KEY', () => { + expect(provider.getRequiredApiKeyName()).toBe('OPENAI_API_KEY'); + }); + }); + + describe('getClient', () => { + it('should throw error if API key is missing', () => { + expect(() => provider.getClient({})).toThrow(Error); + }); + + it('should create client with apiKey only', () => { + const params = { + apiKey: 'sk-test-123' + }; + + // The getClient method should return a function + const client = provider.getClient(params); + expect(typeof client).toBe('function'); + + // The client function should be callable and return a model object + const model = client('gpt-4'); + expect(model).toBeDefined(); + expect(model.modelId).toBe('gpt-4'); + }); + + it('should create client with apiKey and baseURL', () => { + const params = { + apiKey: 'sk-test-456', + baseURL: 'https://api.openai.example' + }; + + // Should not throw when baseURL is provided + const client = provider.getClient(params); + expect(typeof client).toBe('function'); + + // The client function should be callable and return a model object + const model = client('gpt-5'); + expect(model).toBeDefined(); + expect(model.modelId).toBe('gpt-5'); + }); + + it('should return the same client instance for the same parameters', () => { + const params = { + apiKey: 'sk-test-789' + }; + + // Multiple calls with same params should work + const client1 = provider.getClient(params); + const client2 = provider.getClient(params); + + expect(typeof client1).toBe('function'); + expect(typeof client2).toBe('function'); + + // Both clients should be able to create models + const model1 = client1('gpt-4'); + const model2 = client2('gpt-4'); + expect(model1.modelId).toBe('gpt-4'); + expect(model2.modelId).toBe('gpt-4'); + }); + + it('should handle different model IDs correctly', () => { + const client = provider.getClient({ apiKey: 'sk-test-models' }); + + // Test with different models + const gpt4 = client('gpt-4'); + expect(gpt4.modelId).toBe('gpt-4'); + + const gpt5 = client('gpt-5'); + expect(gpt5.modelId).toBe('gpt-5'); + + const gpt35 = client('gpt-3.5-turbo'); + expect(gpt35.modelId).toBe('gpt-3.5-turbo'); + }); + }); + + describe('name property', () => { + it('should have OpenAI as the provider name', () => { + expect(provider.name).toBe('OpenAI'); + }); + }); +});