Skip to content

refine rule criteria, move mode and message to db config #1396

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

Merged
iceljc merged 1 commit into from
Aug 12, 2026
Merged

refine rule criteria, move mode and message to db config #1396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions src/Infrastructure/BotSharp.Abstraction/Agents/Models/AgentRule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,29 @@ public class AgentRule
[JsonPropertyName("disabled")]
public bool Disabled { get; set; }

[JsonPropertyName("config")]
/// <summary>
/// Message sent to agent
/// </summary>
[JsonPropertyName("message")]
public string? Message { get; set; }

[JsonPropertyName("criteria")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public RuleConfig? Config { get; set; }
public RuleCriteria? Criteria { get; set; }
}

public class RuleConfig
public class RuleCriteria
{
/// <summary>
/// Criteria mode: llm, python script, etc.
/// Takes precedence over the mode carried on the trigger options.
/// </summary>
[JsonPropertyName("mode")]
public string? Mode { get; set; }

/// <summary>
/// Criteria text
/// </summary>
[JsonPropertyName("criteria")]
public string? Criteria { get; set; }
}
}
2 changes: 1 addition & 1 deletion src/Infrastructure/BotSharp.Abstraction/Rules/Constants/BuiltInRuleCriteria.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public static class BuiltInRuleCriteria
/// <summary>
/// Evaluate a code script (e.g. Python) that returns a boolean result.
/// </summary>
public const string Code = "code";
public const string PythonScript = "python_script";

@qodo-code-review qodo-code-review Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

3. Criteria identifier renamed 🐞 Bug ☼ Reliability

The built-in criteria identifier changed from "code" to "python_script" and the evaluator now
advertises only python_script; any persisted/requested mode value "code" will no longer resolve,
causing criteria to be skipped.
Agent Prompt 
## Issue description
`BuiltInRuleCriteria` removed/renamed the prior identifier and `CodeCriteriaEvaluator` now reports `Type = "python_script"`. Evaluator resolution is strict string matching, so legacy `mode: "code"` configurations stop working.

## Issue Context
RuleEngine resolves evaluators by comparing evaluator.Type to the configured mode (`x.Type.IsEqualTo(mode)`). With the rename, older stored configs or API callers using `"code"` will resolve no evaluator.

## Fix Focus Areas
- src/Infrastructure/BotSharp.Abstraction/Rules/Constants/BuiltInRuleCriteria.cs[8-19]
- src/Infrastructure/BotSharp.Core.Rules/Criteria/Code/CodeCriteriaEvaluator.cs[24-34]
- src/Infrastructure/BotSharp.Core.Rules/Engines/RuleEngine.cs[79-88]

### Suggested implementation direction
- Add a legacy alias constant (e.g. `public const string Code = "code";`) and/or map `"code"` -> `"python_script"` inside `ResolveCriteriaEvaluator`.
- Alternatively, let the evaluator advertise both identifiers (e.g. by matching on a small set) via resolver mapping logic.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


/// <summary>
/// Ask an LLM whether the rule applies to the request.
Expand Down
5 changes: 0 additions & 5 deletions src/Infrastructure/BotSharp.Abstraction/Rules/Models/RuleCriteriaContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@ namespace BotSharp.Abstraction.Rules.Models;
/// </summary>
public class RuleCriteriaContext
{
/// <summary>
/// The trigger message text.
/// </summary>
public string Text { get; set; } = string.Empty;

/// <summary>
/// The criteria options (evaluator type and its arguments).
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Infrastructure/BotSharp.Abstraction/Rules/Options/RuleTriggerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class CriteriaOptions
/// How the criteria is evaluated (see <see cref="BuiltInRuleCriteria"/>).
/// Selects which <c>IRuleCriteriaEvaluator</c> handles this criteria.
/// </summary>
public string Type { get; set; } = BuiltInRuleCriteria.Code;
public string? Mode { get; set; }

@qodo-code-review qodo-code-review Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

2. Missing default criteria mode 🐞 Bug ≡ Correctness

CriteriaOptions no longer has a default mode, and ResolveCriteriaEvaluator returns null for blank
mode; when callers omit the mode (or still send legacy fields), the engine skips EvaluateCriteria
and still triggers the rule.
Agent Prompt 
## Issue description
`CriteriaOptions` changed from a non-null `Type` with a default value to a nullable `Mode` with no default. In RuleEngine, a null/blank mode resolves no evaluator, and criteria evaluation is skipped while rule triggering continues.

## Issue Context
Previously, `CriteriaOptions.Type` defaulted to the built-in code criteria type. Now, `Mode` can be omitted by callers (including older clients), and the system can silently stop applying criteria.

## Fix Focus Areas
- src/Infrastructure/BotSharp.Abstraction/Rules/Options/RuleTriggerOptions.cs[20-33]
- src/Infrastructure/BotSharp.Core.Rules/Engines/RuleEngine.cs[32-41]
- src/Infrastructure/BotSharp.Core.Rules/Engines/RuleEngine.cs[79-88]

### Suggested implementation direction
Choose at least one:
1) Give `CriteriaOptions.Mode` a safe default (e.g. `BuiltInRuleCriteria.Llm` or `BuiltInRuleCriteria.PythonScript`, depending on intended behavior).
2) Add backward-compatible JSON aliasing (e.g., keep an obsolete `Type` property mapped from `"type"` and translate it into `Mode`).
3) If Mode is absent, fail closed (do not trigger) when criteria evaluation is expected.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


/// <summary>
/// Evaluator-specific settings, kept as raw JSON so each evaluator can
Expand Down
2 changes: 1 addition & 1 deletion src/Infrastructure/BotSharp.Core.Rules/Criteria/Code/CodeCriteriaEvaluator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public CodeCriteriaEvaluator(
_codingSettings = codingSettings;
}

public string Type => BuiltInRuleCriteria.Code;
public string Type => BuiltInRuleCriteria.PythonScript;

public async Task<bool?> EvaluateAsync(Agent agent, IRuleTrigger trigger, RuleCriteriaContext context)
{
Expand Down
8 changes: 4 additions & 4 deletions src/Infrastructure/BotSharp.Core.Rules/Criteria/Llm/LlmCriteriaEvaluator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public LlmCriteriaEvaluator(
var templateName = !string.IsNullOrWhiteSpace(settings.TemplateName)
? settings.TemplateName! : (agentId == BuiltInAgentId.RulesInterpreter ? DefaultTemplateName : $"{trigger.Name}_criteria");

var input = BuildInput(rule?.Config, settings);
var input = BuildInput(rule?.Criteria, settings);
var msg = $"rule trigger ({trigger.Name}) llm criteria (agent {agentId}, template {templateName}).";

try
Expand Down Expand Up @@ -129,14 +129,14 @@ private static Dictionary<string, object> BuildRenderData(RuleCriteriaContext co
return data;
}

private static string BuildInput(RuleConfig? ruleConfig, LlmCriteriaSettings settings)
private static string BuildInput(RuleCriteria? ruleCriteria, LlmCriteriaSettings settings)
{
var sb = new StringBuilder();

if (!string.IsNullOrWhiteSpace(ruleConfig?.Criteria))
if (!string.IsNullOrWhiteSpace(ruleCriteria?.Criteria))
{
sb.AppendLine("## Rule");
sb.AppendLine(ruleConfig.Criteria);
sb.AppendLine(ruleCriteria.Criteria);
sb.AppendLine();
}

Expand Down
Loading
Loading