Merge pull request #544 from majiayu000/fix/issue-529-max-completion-tokens

fix: use max_completion_tokens for reasoning models (o1/o3/o4/gpt-5)

Author: di-sukharev

jest.config.ts

@@ -19,7 +19,7 @@ const config: Config = {
     '<rootDir>/test/e2e/prompt-module/data/'
   ],
   transformIgnorePatterns: [
-    'node_modules/(?!(cli-testing-library|@clack|cleye)/.*)'
+    'node_modules/(?!(cli-testing-library|@clack|cleye|chalk)/.*)'
   ],
   transform: {
     '^.+\\.(ts|tsx|js|jsx|mjs)$': [

src/engine/openAi.ts

@@ -43,12 +43,18 @@ export class OpenAiEngine implements AiEngine {
   public generateCommitMessage = async (
     messages: Array<OpenAI.Chat.Completions.ChatCompletionMessageParam>
   ): Promise<string | null> => {
+    const isReasoningModel = /^(o[1-9]|gpt-5)/.test(this.config.model);
+
     const params = {
       model: this.config.model,
       messages,
-      temperature: 0,
-      top_p: 0.1,
-      max_tokens: this.config.maxTokensOutput
+      ...(isReasoningModel
+        ? { max_completion_tokens: this.config.maxTokensOutput }
+        : {
+            temperature: 0,
+            top_p: 0.1,
+            max_tokens: this.config.maxTokensOutput
+          })
     };
 
     try {
@@ -62,7 +68,9 @@ export class OpenAiEngine implements AiEngine {
       )
         throw new Error(GenerateCommitMessageErrorEnum.tooMuchTokens);
 
-      const completion = await this.client.chat.completions.create(params);
+      const completion = await this.client.chat.completions.create(
+        params as OpenAI.Chat.Completions.ChatCompletionCreateParamsNonStreaming
+      );
 
       const message = completion.choices[0].message;
       let content = message?.content;

test/unit/openAi.test.ts

@@ -0,0 +1,26 @@
+// Test the reasoning model detection regex used in OpenAiEngine.
+// Integration test with the engine is not possible because mistral.ts
+// uses require() which is unavailable in the ESM test environment.
+const REASONING_MODEL_RE = /^(o[1-9]|gpt-5)/;
+
+describe('OpenAiEngine reasoning model detection', () => {
+  it.each([
+    ['o1', true],
+    ['o1-preview', true],
+    ['o1-mini', true],
+    ['o3', true],
+    ['o3-mini', true],
+    ['o4-mini', true],
+    ['gpt-5', true],
+    ['gpt-5-nano', true],
+    ['gpt-4o', false],
+    ['gpt-4o-mini', false],
+    ['gpt-4', false],
+    ['gpt-3.5-turbo', false]
+  ])(
+    'model "%s" isReasoning=%s',
+    (model, expected) => {
+      expect(REASONING_MODEL_RE.test(model)).toBe(expected);
+    }
+  );
+});