Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Clean up local function names in AIFunctionFactory#6909

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
stephentoub merged 9 commits intomainfromcopilot/cleanup-tool-function-names
Oct 10, 2025

Conversation

Copy link
Contributor

CopilotAI commentedOct 10, 2025•
edited
Loading

Summary

Fixed local function name cleanup for AIFunctionFactory, ensuring both lambdas and local functions have unique names.

Changes Made

1. EnhancedSanitizeMemberName in AIFunctionFactory.cs

  • Local functions:<Method>g__LocalFunc|x_y →Method_LocalFunc_x_y (keeps ordinal for uniqueness)
  • Lambdas:<Method>b__x_y →Method_x_y (captures full ordinal suffix for uniqueness)
  • Regular methods: unchanged
  • Fallback: replace non-alphanumeric with underscores
  • Consolidated two separate regexes into a single more general pattern using\w__ for future-proofing

2. Updated Lambda and Local Function Name Handling

  • Changed from two regexes (LocalFunctionNameRegex and LambdaNameRegex) to a single CompilerGeneratedNameRegex
  • Pattern:^<([^>]+)>\w__(.+) matches both local functions and lambdas
  • More future-proof by using\w instead of hardcoding specific letters
  • Keeps ordinal suffix for both local functions and lambdas to prevent name collisions
  • Enhanced "Async" suffix removal to handle cases where ordinal follows "Async" (e.g.,FetchDataAsync_0_0 →FetchData_0_0)
  • Fixed compatibility with netstandard2.0 and net462 by using conditional compilation for string operations

3. Updated Tests

  • Changed all local function and lambda name assertions to useAssert.StartsWith since ordinals vary
  • Improved parameter names in LocalFunction_WithParameters test froma,b tofirstNumber,secondNumber to avoid false positives in schema validation
  • Updated assertions to verify uniqueness usingAssert.NotEqual
  • Added comments to empty test methods to satisfy analyzer requirements (S1186, S108)

Validation Results

✅ All 346 tests passed
✅ Multiple lambdas in same method now get unique names
✅ Multiple local functions with same name in nested scopes now get unique names
✅ No regressions in existing functionality
✅ Build succeeded with 0 warnings and 0 errors (including analyzer checks)
✅Compatible with all target frameworks including netstandard2.0 and net462

Addresses Review Feedback

  • Fixed@stephentoub's concern that multiple lambdas in the same method must have different names to allow the LLM to distinguish which lambda is being requested
  • Addressed potential ambiguity with local functions by keeping ordinal suffixes (nested scopes can have same function names with different ordinals)
  • Fixed analyzer errors (S1186, S108) by adding explanatory comments to empty test method bodies
  • Changed assertions to useAssert.StartsWith for more robust testing with compiler-generated ordinals
  • Consolidated two regexes into one more general pattern for better future-proofing
  • Improved test parameter names to avoid false positives in schema validation
  • Fixed build errors for older target frameworks (netstandard2.0, net462) by using appropriate API overloads

Fixes#6677

Original prompt

This section details on the original issue you should resolve

<issue_title>Local functions used as tool functions could use name cleanup</issue_title>
<issue_description>### Description

There's a bunch of .NET reflection-originating chars in a tool name when it's created from a local function. It would be nice to to additional cleanup to get cleaner tool name by default.

Reproduction Steps

[Fact]publicvoidDefaultToolName(){staticvoidDoSomething(){}vartool=AIFunctionFactory.Create(DoSomething);// renders _DefaultToolName_g__DoSomething_4_0Assert.Equal("DefaultToolName_DoSomething",tool.Name);}

Expected behavior

Since there can't be two members with the same name (neither at the method as well as the local function level), the name could just beDefaultToolName_DoSomething.

Actual behavior

Returns_DefaultToolName_g__DoSomething_4_0

Regression?

No response

Known Workarounds

Set name explicitly

Configuration

net10 preview 6, MEAI 9.7.0

Other information

No response</issue_description>

<agent_instructions>Investigate whether there's a simple approach that employs a simple but robust transformation while also ensuring we don't inadvertently end up with two things with the same name. Any solution should involve a variety of tests that exercise local functions, lambdas, multiple in the same method, etc.</agent_instructions>

Comments on the Issue (you are@copilot in this section)

Fixes#6677

✨ Let Copilot coding agentset things up for you — coding agent works faster and does higher quality work when set up for your repo.

Microsoft Reviewers:Open in CodeFlow

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
CopilotAI changed the title[WIP] Clean up tool function names for clarityClean up local function names in AIFunctionFactoryOct 10, 2025
Copilot finished work on behalf ofstephentoubOctober 10, 2025 00:45
@stephentoub
Copy link
Member

@copilot, there are also build failures in the tests from analyzers:

/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1047,21): error S1186: Add a nested comment explaining why this method is empty, throw a 'NotSupportedException' or complete the implementation. (https://rules.sonarsource.com/csharp/RSPEC-1186) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1048,9): error S108: Either remove or fill this block of code. (https://rules.sonarsource.com/csharp/RSPEC-108) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1060,21): error S1186: Add a nested comment explaining why this method is empty, throw a 'NotSupportedException' or complete the implementation. (https://rules.sonarsource.com/csharp/RSPEC-1186) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1064,21): error S1186: Add a nested comment explaining why this method is empty, throw a 'NotSupportedException' or complete the implementation. (https://rules.sonarsource.com/csharp/RSPEC-1186) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1061,9): error S108: Either remove or fill this block of code. (https://rules.sonarsource.com/csharp/RSPEC-108) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1065,9): error S108: Either remove or fill this block of code. (https://rules.sonarsource.com/csharp/RSPEC-108) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1143,21): error S1186: Add a nested comment explaining why this method is empty, throw a 'NotSupportedException' or complete the implementation. (https://rules.sonarsource.com/csharp/RSPEC-1186) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1144,9): error S108: Either remove or fill this block of code. (https://rules.sonarsource.com/csharp/RSPEC-108) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1047,21): error S1186: Add a nested comment explaining why this method is empty, throw a 'NotSupportedException' or complete the implementation. (https://rules.sonarsource.com/csharp/RSPEC-1186) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1048,9): error S108: Either remove or fill this block of code. (https://rules.sonarsource.com/csharp/RSPEC-108) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1060,21): error S1186: Add a nested comment explaining why this method is empty, throw a 'NotSupportedException' or complete the implementation. (https://rules.sonarsource.com/csharp/RSPEC-1186) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1064,21): error S1186: Add a nested comment explaining why this method is empty, throw a 'NotSupportedException' or complete the implementation. (https://rules.sonarsource.com/csharp/RSPEC-1186) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1061,9): error S108: Either remove or fill this block of code. (https://rules.sonarsource.com/csharp/RSPEC-108) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1065,9): error S108: Either remove or fill this block of code. (https://rules.sonarsource.com/csharp/RSPEC-108) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1143,21): error S1186: Add a nested comment explaining why this method is empty, throw a 'NotSupportedException' or complete the implementation. (https://rules.sonarsource.com/csharp/RSPEC-1186) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1144,9): error S108: Either remove or fill this block of code. (https://rules.sonarsource.com/csharp/RSPEC-108) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]    0 Warning(s)    16 Error(s)

Please make sure build is successful and all tests successfully complete before declaring success.

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copilot finished work on behalf ofstephentoubOctober 10, 2025 01:21
@stephentoubstephentoub marked this pull request as ready for reviewOctober 10, 2025 01:27
@stephentoubstephentoub requested a review froma team as acode ownerOctober 10, 2025 01:27
CopilotAI review requested due to automatic review settingsOctober 10, 2025 01:27
Copy link
Contributor

CopilotAI 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 enhances theSanitizeMemberName method inAIFunctionFactory to provide cleaner names for local functions and lambdas used as AI tools. The changes ensure that compiler-generated names are transformed into more readable formats while maintaining uniqueness for multiple functions in the same method.

  • EnhancedSanitizeMemberName to handle local functions and lambdas with specific regex patterns
  • Added comprehensive tests covering various scenarios including multiple functions in the same method
  • Ensured lambda names retain complete ordinal suffixes for uniqueness

Reviewed Changes

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

FileDescription
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionFactory.csEnhancedSanitizeMemberName method with regex patterns for local functions and lambdas, added new regex methods
test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.csAdded comprehensive test coverage for local functions, lambdas, and edge cases with a helper static method

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copy link
ContributorAuthor

@copilot, there are also build failures in the tests from analyzers:

/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(1047,21): error S1186: Add...

Fixed in commit 3d7e842. Added explanatory comments to all empty test method bodies (local functions and lambdas) to satisfy analyzer requirements S1186 and S108. Build now succeeds with 0 warnings and 0 errors, and all 346 tests pass.

Copilot finished work on behalf ofstephentoubOctober 10, 2025 01:29
@stephentoubstephentoub added the area-aiMicrosoft.Extensions.AI libraries labelOct 10, 2025
@stephentoub
Copy link
Member

@jaredpar, I don't love taking a dependency here on the compiler's implementation detail around naming. Am I right to be nervous, or is what this PR is doing reasonable?

@eiriktsarpalis
Copy link
Member

Am I right to be nervous, or is what this PR is doing reasonable?

What's the worst thing that could happen if the naming convention changes? I take it it would just revert to using the original name.

@stephentoub
Copy link
Member

Am I right to be nervous, or is what this PR is doing reasonable?

What's the worst thing that could happen if the naming convention changes? I take it it would just revert to using the original name.

Probably... though if the compiler's scheme were to change in specific ways, in theory I think we could end up with some duplicated names in corner cases, which could lead to ambiguity in the names provided to the LLM. In practice, though, I don't think that would happen, and really a developer should be providing a better name explicitly.

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copilot finished work on behalf ofstephentoubOctober 10, 2025 16:20
@stephentoub
Copy link
Member

Also,@copilot, there are build failures for netstandard2.0 / net462:

D:\a\_work\1\s\src\Libraries\Microsoft.Extensions.AI.Abstractions\Functions\AIFunctionFactory.cs(1120,53): error CS1503: Argument 2: cannot convert from 'System.StringComparison' to 'int' [D:\a\_work\1\s\src\Libraries\Microsoft.Extensions.AI.Abstractions\Microsoft.Extensions.AI.Abstractions.csproj::TargetFramework=netstandard2.0]D:\a\_work\1\s\src\Libraries\Microsoft.Extensions.AI.Abstractions\Functions\AIFunctionFactory.cs(1120,53): error CS1503: Argument 2: cannot convert from 'System.StringComparison' to 'int' [D:\a\_work\1\s\src\Libraries\Microsoft.Extensions.AI.Abstractions\Microsoft.Extensions.AI.Abstractions.csproj::TargetFramework=net462]

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copy link
ContributorAuthor

Also,@copilot, there are build failures for netstandard2.0 / net462:

D:\a\_work\1\s\src\Libraries\Microsoft.Extensions.AI.Abstractions\Functions\AIFunctionFactory.cs(1120,53): error CS1503: Arg...

Fixed in commit 8b4c7e3. Used conditional compilation to handle the string operations differently for older frameworks. For netstandard2.0/net462, usingIndexOf(string) without StringComparison parameter, andSubstring concatenation instead of span-based operations. Added pragma suppressions for locale warnings since compiler-generated names are ASCII-only.

Copilot finished work on behalf ofstephentoubOctober 10, 2025 18:31
@stephentoubstephentoub merged commit4c8b35f intomainOct 10, 2025
6 checks passed
@stephentoubstephentoub deleted the copilot/cleanup-tool-function-names branchOctober 10, 2025 22:19
joperezr pushed a commit to joperezr/extensions that referenced this pull requestOct 14, 2025
* Initial plan* Implement local function name cleanup for AIFunctionFactoryCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Fix lambda name uniqueness by capturing full ordinal suffixCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Fix analyzer errors by adding comments to empty test methodsCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Change Assert.Contains to Assert.StartsWith for lambda name testsCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Consolidate regexes and improve test parameter namesCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Keep ordinal suffix for local functions to ensure uniquenessCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Fix build errors for netstandard2.0 and net462 targetsCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Cleanup---------Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>Co-authored-by: Stephen Toub <stoub@microsoft.com>
jeffhandley pushed a commit to jeffhandley/extensions that referenced this pull requestOct 21, 2025
* Initial plan* Implement local function name cleanup for AIFunctionFactoryCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Fix lambda name uniqueness by capturing full ordinal suffixCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Fix analyzer errors by adding comments to empty test methodsCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Change Assert.Contains to Assert.StartsWith for lambda name testsCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Consolidate regexes and improve test parameter namesCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Keep ordinal suffix for local functions to ensure uniquenessCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Fix build errors for netstandard2.0 and net462 targetsCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Cleanup---------Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>Co-authored-by: Stephen Toub <stoub@microsoft.com>
jeffhandley pushed a commit to jeffhandley/extensions that referenced this pull requestOct 21, 2025
* Initial plan* Implement local function name cleanup for AIFunctionFactoryCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Fix lambda name uniqueness by capturing full ordinal suffixCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Fix analyzer errors by adding comments to empty test methodsCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Change Assert.Contains to Assert.StartsWith for lambda name testsCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Consolidate regexes and improve test parameter namesCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Keep ordinal suffix for local functions to ensure uniquenessCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Fix build errors for netstandard2.0 and net462 targetsCo-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>* Cleanup---------Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>Co-authored-by: Stephen Toub <stoub@microsoft.com>
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsNov 10, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

Copilot code reviewCopilotCopilot left review comments

@jozkeejozkeejozkee approved these changes

@eiriktsarpaliseiriktsarpalisAwaiting requested review from eiriktsarpalis

+1 more reviewer

@stephentoubstephentoubstephentoub approved these changes

Reviewers whose approvals may not affect merge requirements

Labels

area-aiMicrosoft.Extensions.AI libraries

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Local functions used as tool functions could use name cleanup

4 participants

@stephentoub@eiriktsarpalis@jozkee
[8]ページ先頭
©2009-2025 Movatter.jp