Skip to content

UserInput Request/Response: rename id to requestId #7106

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.

Sign up for GitHub

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

Jump to bottom
Open
jozkee wants to merge 1 commit into
base: main
Choose a base branch
from
Open

UserInput Request/Response: rename id to requestId #7106

jozkee wants to merge 1 commit into from

Conversation

@jozkee
Copy link
Member

@jozkee jozkee commented Dec 4, 2025 •
edited by dotnet-policy-service bot
Loading

Id is too vague, changing to RequestId in both approval request/response contents.
Related to #6492

The naming of "id" parameters is confusing; it's not clear which ID they're referring to, and more than once I passed in the wrong ID. We should revisit the naming.

@westey-m

Microsoft Reviewers: Open in CodeFlow

@jozkee jozkee requested review from Copilot and stephentoub December 4, 2025 22:15
@jozkee jozkee requested a review from a team as a code owner December 4, 2025 22:15
@github-actions github-actions bot added the area-ai Microsoft.Extensions.AI libraries label Dec 4, 2025
@stephentoub
Copy link
Member

stephentoub commented Dec 4, 2025

The naming of "id" parameters is confusing; it's not clear which ID they're referring to, and more than once I passed in the wrong ID. We should revisit the naming.

I'm curious which other ID this was confused with?

stephentoub
stephentoub reviewed Dec 4, 2025
View reviewed changes
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
// Validation: Capture each call id for each approval request to ensure later we have a matching response.
_ = (approvalRequestCallIds ??= []).Add(farc.FunctionCall.CallId);
(allApprovalRequestsMessages ??= []).Add(farc.Id, message);
(allApprovalRequestsMessages ??= []).Add(farc.RequestId, message);
Copy link
Member

@stephentoub stephentoub Dec 4, 2025

Choose a reason for hiding this comment

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

Let's be thoughtful of when we merge this change with regards to next week's release and any other changes to this same area. It'd be good for all of the breaking changes we plan to make to all of this [Experimental] surface area to land in the same release.

Copy link
Member Author

@jozkee jozkee Dec 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It probably won't make it for next week, unless we land these other proposed changes:

Copilot AI reviewed Dec 4, 2025
View reviewed changes
Copy link
Contributor

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

This PR improves API clarity by renaming the ambiguous id parameter to requestId in the UserInput Request/Response API classes. The change addresses issue #6492 where the generic "id" naming was confusing, making it unclear which ID was being referenced. Since these APIs are marked as experimental ([Experimental("MEAI001")]), this is an appropriate time to improve the naming before stabilization.

Key changes:

  • Renamed Id property to RequestId in UserInputRequestContent and UserInputResponseContent base classes
  • Updated all derived classes: FunctionApprovalRequestContent, FunctionApprovalResponseContent, McpServerToolApprovalRequestContent, and McpServerToolApprovalResponseContent
  • Updated usages in FunctionInvokingChatClient and OpenAIResponsesChatClient

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/UserInputRequestContent.cs Renamed constructor parameter id to requestId and property Id to RequestId in base class
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/UserInputResponseContent.cs Renamed constructor parameter id to requestId and property Id to RequestId in base class
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionApprovalRequestContent.cs Updated constructor parameter and CreateResponse method to use RequestId
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionApprovalResponseContent.cs Updated constructor parameter and documentation to use requestId
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/McpServerToolApprovalRequestContent.cs Updated constructor parameter and CreateResponse method to use RequestId
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/McpServerToolApprovalResponseContent.cs Updated constructor parameter and documentation to use requestId
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs Updated property access from .Id to .RequestId
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs Updated property access from .Id to .RequestId in three locations
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/UserInputRequestContentTests.cs Updated test parameter names and assertions
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/UserInputResponseContentTests.cs Updated test parameter names and assertions
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionApprovalRequestContentTests.cs Updated test parameter names and assertions
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionApprovalResponseContentTests.cs Updated test parameter names and assertions

@jozkee
Copy link
Member Author

jozkee commented Dec 4, 2025

I'm curious which other ID this was confused with?

Most likely the CallId from FunctionCall/ResultContent and McpServerToolCall/ResultContent. It was originally mentioned in #6779, in case that rings a bell.

@stephentoub
Copy link
Member

stephentoub commented Dec 4, 2025

I'm curious which other ID this was confused with?

Most likely the CallId from FunctionCall/ResultContent and McpServerToolCall/ResultContent. It was originally mentioned in #6779, in case that rings a bell.

Ah, ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stephentoub stephentoub stephentoub approved these changes

Copilot code review Copilot Copilot left review comments

Assignees

@jozkee jozkee

Labels

area-ai Microsoft.Extensions.AI libraries

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jozkee @stephentoub