fix(routing): normalize function name, fix length-truncated response …#1291
fix(routing): normalize function name, fix length-truncated response …#1291yileicn merged 2 commits intoSciSharp:masterfrom
Conversation
…handling, add DiagnosticHelper
Review Summary by QodoNormalize function names, handle length-truncated responses, and add diagnostic utilities
WalkthroughsDescription• Add NormalizeFunctionName() extension method to handle LLM-generated function names with namespace/agent prefixes • Create DiagnosticHelper utility class for stack trace debugging • Fix length-truncated response handling across multiple chat completion providers • Consolidate function name normalization logic across 6 provider implementations • Improve error logging with diagnostic stack trace information Diagramflowchart LR
A["LLM Response with<br/>Prefixed Function Names"] -->|NormalizeFunctionName| B["Cleaned Function Names"]
C["Length-Truncated<br/>Responses"] -->|ChatFinishReason.Length| D["Handled with<br/>StopCompletion Flag"]
E["Function Execution<br/>Failures"] -->|DiagnosticHelper| F["Enhanced Error Logs<br/>with Stack Trace"]
B --> G["6 Chat Providers<br/>Updated"]
D --> G
F --> G
File Changes1. src/Infrastructure/BotSharp.Abstraction/Utilities/DiagnosticHelper.cs
|
Code Review by Qodo
1. Stack trace logged in error
|
| message.StopCompletion = true; | ||
| message.Content = $"Can't find function implementation of {name}."; | ||
| _logger.LogError(message.Content); | ||
| _logger.LogError($"{message.Content}, stackInfo:{DiagnosticHelper.GetCurrentStackTrace()}"); | ||
| return false; |
There was a problem hiding this comment.
1. Stack trace logged in error 📘 Rule violation ⛨ Security
• The new error log appends a full stack trace via DiagnosticHelper.GetCurrentStackTrace() when a function implementation is missing. • Stack traces can contain sensitive internal details (method names, paths, code locations) and the log line is also unstructured, making it harder to audit safely. • This increases the risk of sensitive information exposure through logs and reduces log usability for monitoring/auditing.
Agent Prompt
## Issue description
The code logs a full stack trace as part of an interpolated error message. This can leak internal details into logs and is not structured logging.
## Issue Context
Secure logging requires logs to be structured and to avoid sensitive/internal details in log output.
## Fix Focus Areas
- src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeFunction.cs[19-22]
- src/Infrastructure/BotSharp.Abstraction/Utilities/DiagnosticHelper.cs[12-15]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| public static string? NormalizeFunctionName(this string? functionName) | ||
| { | ||
| if (string.IsNullOrEmpty(functionName)) | ||
| { | ||
| return functionName; | ||
| } | ||
|
|
||
| if (functionName.Contains('.')) | ||
| { | ||
| return functionName.Split('.').Last(); | ||
| } | ||
|
|
||
| if (functionName.Contains('/')) | ||
| { | ||
| return functionName.Split('/').Last(); | ||
| } |
There was a problem hiding this comment.
2. Normalization can yield invalid name 🐞 Bug ✓ Correctness
• NormalizeFunctionName can return empty string or whitespace (e.g., inputs like "Agent." or "Agent/ ") because it doesn’t trim or remove empty segments. • Multiple call sites now assign the normalized value back to FunctionName/args.Function without validation, increasing the chance of passing an empty/invalid function name downstream. • Function execution is resolved by exact name equality; an empty/whitespace name will fail lookup and surface as “Can’t find function implementation …”, making failures harder to attribute to malformed LLM output.
Agent Prompt
## Issue description
`NormalizeFunctionName` can normalize malformed names into empty/whitespace strings (e.g., trailing `.` or `/`, leading/trailing spaces). Those values then fail downstream exact-match lookups for executors.
## Issue Context
Function execution is resolved via `x.Name == functionName` (exact equality). Call sites now overwrite the function name with the normalized value without validating it.
## Fix Focus Areas
- src/Infrastructure/BotSharp.Abstraction/Utilities/StringExtensions.cs[74-92]
- src/Infrastructure/BotSharp.Core/Routing/Executor/FunctionExecutorFactory.cs[8-18]
## Suggested direction
- Update `NormalizeFunctionName` to:
- use `string.IsNullOrWhiteSpace` for the initial guard
- `Trim()` the input
- split on both `.` and `/` (e.g., via `Split(new[]{'.','/'}, StringSplitOptions.RemoveEmptyEntries)`)
- return the last non-empty segment (trimmed)
- if the normalized token is empty, return `null` (or original) so callers can handle it
- Optionally add a small validation at call sites: if normalized is null/whitespace, do not overwrite and/or set a clear error explaining malformed tool name.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
reviewed |
fix(routing): normalize function name, fix length-truncated response handling, add DiagnosticHelper