Skip to content

Conversation

@iceljc
Copy link
Collaborator

@iceljc iceljc commented Oct 17, 2025

PR Type

Enhancement


Description

  • Add BeforeCompletion hook before code execution

  • Add AfterCompletion hook after code execution

  • Extend hook lifecycle with completion callbacks


Diagram Walkthrough

flowchart LR
  A["Instruction Execution"] --> B["BeforeCompletion Hook"]
  B --> C["BeforeCodeExecution Hook"]
  C --> D["Code Execution"]
  D --> E["AfterCodeExecution Hook"]
  E --> F["AfterCompletion Hook"]
  F --> G["Response Generated"]
Loading

File Walkthrough

Relevant files
Enhancement
InstructService.Execute.cs
Add BeforeCompletion and AfterCompletion hooks                     

src/Infrastructure/BotSharp.Core/Instructs/Services/InstructService.Execute.cs

  • Added await hook.BeforeCompletion(agent, message) call before code
    execution
  • Added await hook.AfterCompletion(agent, response) call after code
    execution
  • Extended the hook lifecycle to include completion callbacks at
    strategic points
+2/-0     

@iceljc iceljc merged commit d1df7f7 into SciSharp:master Oct 17, 2025
0 of 4 checks passed
@qodo-merge-pro
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 <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

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

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent null reference exception on response

Add a null check for the response variable before the "After code execution"
loop to prevent a potential NullReferenceException.

src/Infrastructure/BotSharp.Core/Instructs/Services/InstructService.Execute.cs [270-285]

 // After code execution
-foreach (var hook in hooks)
+if (response != null)
 {
-    await hook.AfterCompletion(agent, response);
-    await hook.AfterCodeExecution(agent, response);
-    await hook.OnResponseGenerated(new InstructResponseModel
+    foreach (var hook in hooks)
     {
-        AgentId = agent.Id,
-        Provider = codeProcessor.Provider,
-        Model = string.Empty,
-        TemplateName = scriptName,
-        UserMessage = message.Content,
-        SystemInstruction = context?.CodeScript,
-        CompletionText = response.Text
-    });
+        await hook.AfterCompletion(agent, response);
+        await hook.AfterCodeExecution(agent, response);
+        await hook.OnResponseGenerated(new InstructResponseModel
+        {
+            AgentId = agent.Id,
+            Provider = codeProcessor.Provider,
+            Model = string.Empty,
+            TemplateName = scriptName,
+            UserMessage = message.Content,
+            SystemInstruction = context?.CodeScript,
+            CompletionText = response.Text
+        });
+    }
 }
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential NullReferenceException when accessing response.Text, as response can be null. Adding a null check is crucial to prevent a runtime crash.

Medium
  • More

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.

1 participant