Skip to content

chat: fix enterprise custom GPT-5 requests - #328458

Draft
Vritant Bhardwaj (vritant24) wants to merge 4 commits into
mainfrom
agents/investigate-unit-tests-issue-reproduction
Draft

chat: fix enterprise custom GPT-5 requests#328458
Vritant Bhardwaj (vritant24) wants to merge 4 commits into
mainfrom
agents/investigate-unit-tests-issue-reproduction

Conversation

@vritant24

@vritant24 Vritant Bhardwaj (vritant24) commented Jul 31, 2026

Copy link
Copy Markdown
Member

Fixes #328418

GitHub-hosted enterprise custom GPT-5 models configured with the Chat Completions API were sent max_tokens. Microsoft Foundry rejects that parameter for these models and requires max_completion_tokens, causing requests to fail with HTTP 400.

This change:

  • maps max_tokens to max_completion_tokens for hosted custom GPT-5 thinking models using Chat Completions
  • leaves built-in, non-thinking, non-GPT, Responses API, and Messages API requests unchanged
  • adds regression coverage for the reported GPT-5.1, GPT-5.4, and GPT-5.6-terra configurations

Validation:

  • Copilot extension typecheck
  • 31 targeted unit tests
  • hygiene checks

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

The thinking-capability guard leaves non-thinking custom GPT-5 endpoints sending an unsupported parameter.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Updates GitHub-hosted custom GPT-5 Chat Completions requests to use the required token parameter.

Changes:

  • Converts max_tokens to max_completion_tokens.
  • Adds positive and negative characterization tests.
  • Narrows the apiType return type.
File summaries
File Description
chatEndpoint.ts Adds GPT-5 custom-model request transformation.
copilotChatEndpoint.spec.ts Tests token parameter handling across model variants.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread extensions/copilot/src/platform/endpoint/node/chatEndpoint.ts Outdated
@vritant24 Vritant Bhardwaj (vritant24) changed the title Investigate and add unit tests for Chat Completions issue chat: fix enterprise custom GPT-5 requests Jul 31, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

Regression coverage is missing for the Responses and Messages API exclusion paths.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

extensions/copilot/src/platform/endpoint/node/chatEndpoint.ts:347

  • Please add regression cases for custom thinking GPT-5 models using the Responses and Messages APIs. This new gate is the only protection for those request shapes, but the negative table exercises only model traits, so removing or miscomputing the API check would leave all current tests passing despite the stated requirement that those APIs remain unchanged.
			&& this.apiType === 'chatCompletions'
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Default server-provided custom Chat Completions models to max_completion_tokens, with an advanced experiment-based compatibility override for endpoints that still require max_tokens. Fixes #328418.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move the experiment-based compatibility setting into the advanced manifest section while keeping its public setting ID free of the internal .advanced. prefix.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

BYOK via Github Copilot Chat - 400 error on max-completion-token

3 participants