Skip to content

Conversation

@yileicn
Copy link
Member

@yileicn yileicn commented Dec 31, 2025

PR Type

Enhancement


Description

  • Replace Task.WaitAll with async/await foreach loops

  • Convert synchronous methods to async Task-returning methods

  • Update test mock implementations to match async signatures


Diagram Walkthrough

flowchart LR
  A["Task.WaitAll with Select"] -->|"Replace with"| B["async foreach + await"]
  C["Sync Methods"] -->|"Convert to"| D["Async Task Methods"]
  E["Test Mocks"] -->|"Update signatures"| F["Async Task Returns"]
Loading

File Walkthrough

Relevant files
Enhancement
TextCompletionProvider.cs
Replace Task.WaitAll with async foreach loops                       

src/Plugins/BotSharp.Plugin.AzureOpenAI/Providers/Text/TextCompletionProvider.cs

  • Replace Task.WaitAll with foreach loop and await for BeforeGenerating
    hook
  • Replace Task.WaitAll with foreach loop and await for AfterGenerated
    hook
  • Improve code readability and async/await pattern consistency
+16/-12 
TextCompletionProvider.cs
Replace Task.WaitAll with async foreach loops                       

src/Plugins/BotSharp.Plugin.OpenAI/Providers/Text/TextCompletionProvider.cs

  • Replace Task.WaitAll with foreach loop and await for BeforeGenerating
    hook
  • Replace Task.WaitAll with foreach loop and await for AfterGenerated
    hook
  • Improve async code pattern and readability
+17/-12 
TencentCosService.User.cs
Convert user avatar methods to async                                         

src/Plugins/BotSharp.Plugin.TencentCos/Services/TencentCosService.User.cs

  • Convert GetUserAvatar() from synchronous to async Task
  • Convert SaveUserAvatar() from synchronous to async Task
  • Add await keyword for db.GetUserById() calls to support async
    operations
+4/-4     
Tests
NullConversationStateService.cs
Update conversation state service to async                             

tests/BotSharp.LLM.Tests/Core/NullConversationStateService.cs

  • Convert Load() method to return Task>
  • Convert Save() method to return Task instead of void
  • Wrap return values with Task.FromResult() and Task.CompletedTask
+4/-4     
NullFileStorageService.cs
Update file storage mock to async signatures                         

tests/BotSharp.LLM.Tests/Core/NullFileStorageService.cs

  • Convert GetUserAvatar() to return Task
  • Convert SaveUserAvatar() to return Task
  • Wrap return values with Task.FromResult()
+4/-4     
TestAgentService.cs
Update agent service mock to async                                             

tests/BotSharp.LLM.Tests/Core/TestAgentService.cs

  • Convert GetPlugin() method to return Task
  • Wrap return value with Task.FromResult()
+2/-2     

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Execute hooks concurrently for better performance

Replace the sequential await in the foreach loop with Task.WhenAll to execute
BeforeGenerating hooks concurrently, improving performance.

src/Plugins/BotSharp.Plugin.AzureOpenAI/Providers/Text/TextCompletionProvider.cs [53-60]

-foreach (var hook in contentHooks)
-{
-    await hook.BeforeGenerating(agent,
-        new List<RoleDialogModel>
-        {
-            message
-        });
-}
+var beforeGeneratingTasks = contentHooks.Select(hook =>
+    hook.BeforeGenerating(agent, new List<RoleDialogModel> { message }));
+await Task.WhenAll(beforeGeneratingTasks);
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that changing from Task.WaitAll to a sequential foreach loop with await is a performance regression. Proposing Task.WhenAll restores concurrency in a non-blocking, asynchronous way, which is a significant improvement.

Medium
Performance
Parallelize hook execution

Replace the sequential await in the foreach loops with Task.WhenAll to execute
all hooks concurrently, improving performance.

src/Plugins/BotSharp.Plugin.OpenAI/Providers/Text/TextCompletionProvider.cs [42-84]

-foreach (var hook in contentHooks)
-{
-    await hook.BeforeGenerating(agent,
-        new List<RoleDialogModel>
-        {
-            message
-        });
-}
+await Task.WhenAll(contentHooks.Select(hook =>
+    hook.BeforeGenerating(agent,
+        new List<RoleDialogModel> { message })));
 
-foreach (var hook in contentHooks)
-{
-    await hook.AfterGenerated(responseMessage, new TokenStatsModel
+await Task.WhenAll(contentHooks.Select(hook =>
+    hook.AfterGenerated(responseMessage, new TokenStatsModel
     {
         Prompt = text,
         Provider = Provider,
         Model = _model,
         TextInputTokens = response.Usage?.PromptTokens ?? 0,
         TextOutputTokens = response.Usage?.CompletionTokens ?? 0
-    });
-}
+    })));
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that the PR's change from Task.WaitAll to a sequential await in a loop can degrade performance. Using Task.WhenAll is the correct asynchronous pattern to run independent tasks concurrently, improving efficiency.

Medium
  • More

@JackJiang1234
Copy link
Contributor

reviewed

@yileicn yileicn merged commit a982fdc into SciSharp:master Dec 31, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants