Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
This comment has been minimized.
This comment has been minimized.
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe chatbot now supports expanded responder, call-reference, department-switching, status, and staffing phrases, resolves calls by multiple reference types, and provides topic-based help. STOP handling performs SMS opt-out without ending sessions. Chatbot SMS replies use a configurable length override. ChangesChatbot interactions
STOP opt-out handling
Chatbot SMS length controls
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant KeywordIntentClassifier
participant ChatbotIngressService
participant CallReferenceResolver
participant ICallsService
User->>KeywordIntentClassifier: send responder or call-reference text
KeywordIntentClassifier->>ChatbotIngressService: return classified intent and reference
ChatbotIngressService->>CallReferenceResolver: resolve reference for department
CallReferenceResolver->>ICallsService: search call ID, number, or shorthand
ICallsService-->>CallReferenceResolver: matching call or no match
CallReferenceResolver-->>ChatbotIngressService: resolved call result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Core/Resgrid.Chatbot.NLU/Providers/KeywordIntentClassifier.cs`:
- Around line 185-187: Update the matching flow around KeywordIntentClassifier’s
trimmed input so trailing punctuation is removed only from a separate normalized
copy used for anchored intent matching. Continue extracting free-form parameters
and dispatch descriptions from the original trimmed input, preserving
punctuation such as question marks and periods.
In `@Core/Resgrid.Chatbot/Handlers/HelpActionHandler.cs`:
- Around line 72-77: The help output built in HelpActionHandler should advertise
classifier-compatible commands: change custom status entries to use the “SET
STATUS TO <name>” form, and replace the bare “MSG” help entry with “MESSAGES”.
Update both affected help-generation branches while preserving their existing
labels and ordering.
In `@Core/Resgrid.Chatbot/Services/CallReferenceResolver.cs`:
- Around line 53-60: Update the shorthand resolution logic in
CallReferenceResolver to detect when multiple active calls match the term
instead of selecting the newest via OrderByDescending(...).FirstOrDefault().
Require a single match or return an explicit ambiguity result that
RespondToCallHandler can use to prompt the user for a call number, while
preserving direct resolution for exactly one match.
In `@Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs`:
- Around line 419-438: Move the ClassifyIntentAsync call and the complete
intent.Type == ChatbotIntentType.Stop handling block before the session
state-machine logic, such as immediately before session continuation begins.
Preserve the existing SMS opt-out behavior through
DisableTextMessagesForUserAsync and the non-SMS response, ensuring STOP returns
before any session-state processing.
- Around line 259-271: Update the restricted-mode STOP handling in
ChatbotIngressService so non-SMS platforms return the same notification opt-out
guidance as the normal-mode STOP branch, while preserving the existing SMS
disable-and-response behavior. Ensure non-SMS STOP exits before the
switchableDepartments flow and uses the established response text and fields
from the normal-mode handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5cf38edc-e839-4716-ab61-d724cc15834a
⛔ Files ignored due to path filters (2)
Tests/Resgrid.Tests/Chatbot/ChatbotHandlerTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Chatbot/ChatbotSecurityTests.csis excluded by!**/Tests/**
📒 Files selected for processing (12)
Core/Resgrid.Chatbot.NLU/Providers/KeywordIntentClassifier.csCore/Resgrid.Chatbot/Handlers/CallDetailActionHandler.csCore/Resgrid.Chatbot/Handlers/HelpActionHandler.csCore/Resgrid.Chatbot/Handlers/RespondToCallHandler.csCore/Resgrid.Chatbot/Localization/ChatbotResources.csCore/Resgrid.Chatbot/Services/CallReferenceResolver.csCore/Resgrid.Chatbot/Services/ChatbotIngressService.csCore/Resgrid.Config/ChatbotConfig.csCore/Resgrid.Model/Providers/ITextMessageProvider.csCore/Resgrid.Services/TextCommandService.csProviders/Resgrid.Providers.Number/TextMessageProvider.csWorkers/Resgrid.Workers.Framework/Logic/ChatbotMessageLogic.cs
| // Texters end questions/commands with punctuation ("My status?", "omw to 26-1."). The | ||
| // patterns are anchored, so strip trailing punctuation before matching. | ||
| trimmed = trimmed.TrimEnd('?', '!', '.', ',', ' '); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve punctuation in extracted free-form parameters.
Sanitizing trimmed globally removes meaningful punctuation from message bodies and dispatch descriptions; for example, send message to Sam: Are you okay? sends Are you okay. Match against a normalized copy while extracting parameters from the original trimmed input.
Proposed approach
- trimmed = trimmed.TrimEnd('?', '!', '.', ',', ' ');
- if (trimmed.Length == 0)
+ var normalized = trimmed.TrimEnd('?', '!', '.', ',', ' ');
+ if (normalized.Length == 0)
return Task.FromResult(new NLUResult { IntentName = "unknown", Confidence = 0, ProviderName = ProviderName });
foreach (var (pattern, intent, extractor) in _patterns)
{
- var match = pattern.Match(trimmed);
+ var match = pattern.Match(normalized);Parameter-bearing patterns should then preserve trailing punctuation explicitly or only normalize command/query intents.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Texters end questions/commands with punctuation ("My status?", "omw to 26-1."). The | |
| // patterns are anchored, so strip trailing punctuation before matching. | |
| trimmed = trimmed.TrimEnd('?', '!', '.', ',', ' '); | |
| var normalized = trimmed.TrimEnd('?', '!', '.', ',', ' '); | |
| if (normalized.Length == 0) | |
| return Task.FromResult(new NLUResult { IntentName = "unknown", Confidence = 0, ProviderName = ProviderName }); | |
| foreach (var (pattern, intent, extractor) in _patterns) | |
| { | |
| var match = pattern.Match(normalized); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Resgrid.Chatbot.NLU/Providers/KeywordIntentClassifier.cs` around lines
185 - 187, Update the matching flow around KeywordIntentClassifier’s trimmed
input so trailing punctuation is removed only from a separate normalized copy
used for anchored intent matching. Continue extracting free-form parameters and
dispatch descriptions from the original trimmed input, preserving punctuation
such as question marks and periods.
| var customActions = await GetCustomStateAsync(departmentId, CustomStateTypes.Personnel); | ||
| if (customActions != null) | ||
| { | ||
| var details = customActions.GetActiveDetails(); | ||
| for (int i = 0; i < details.Count; i++) | ||
| sb.AppendLine($"{i + 1} or {details[i].ButtonText?.Replace(" ", "")}"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Only advertise commands recognized by the classifier.
Two help entries currently produce unsupported input:
- Custom status labels are shown as bare tokens, but arbitrary labels require
SET STATUS TO <name>. - Bare
MSGdoes not match the list-messages patterns; useMESSAGES.
Proposed fix
- sb.AppendLine($"{i + 1} or {details[i].ButtonText?.Replace(" ", "")}");
+ sb.AppendLine($"{i + 1} or SET STATUS TO {details[i].ButtonText}");
...
- + "MSG: list your messages\n"
+ + "MESSAGES: list your messages\n"Also applies to: 126-130
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Resgrid.Chatbot/Handlers/HelpActionHandler.cs` around lines 72 - 77, The
help output built in HelpActionHandler should advertise classifier-compatible
commands: change custom status entries to use the “SET STATUS TO <name>” form,
and replace the bare “MSG” help entry with “MESSAGES”. Update both affected
help-generation branches while preserving their existing labels and ordering.
| // 3. Shorthand ("fire"): term appears in the name, type, or nature of an active call; | ||
| // most recent match wins. | ||
| return activeCalls | ||
| .Where(c => ContainsTerm(c.Name, text) | ||
| || ContainsTerm(c.Type, text) | ||
| || ContainsTerm(c.NatureOfCall == null ? null : StringHelpers.StripHtmlTagsCharArray(c.NatureOfCall), text)) | ||
| .OrderByDescending(c => c.LoggedOn) | ||
| .FirstOrDefault(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File list ==\n'
git ls-files 'Core/Resgrid.Chatbot/Services/CallReferenceResolver.cs' \
'Core/Resgrid.Chatbot/Handlers/RespondToCallHandler.cs' \
'Core/Resgrid.Chatbot/**/*Call*' \
'Core/Resgrid.Chatbot/**/*Respond*' | sed 's#^`#-` #'
printf '\n== Outline: CallReferenceResolver ==\n'
ast-grep outline Core/Resgrid.Chatbot/Services/CallReferenceResolver.cs --view expanded || true
printf '\n== Outline: RespondToCallHandler ==\n'
ast-grep outline Core/Resgrid.Chatbot/Handlers/RespondToCallHandler.cs --view expanded || true
printf '\n== Search for resolver usage ==\n'
rg -n "CallReferenceResolver|Resolve.*Call|activeCalls|Shorthand|fire" Core/Resgrid.Chatbot -SRepository: Resgrid/Core
Length of output: 6287
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'Core/Resgrid.Chatbot/Services/CallReferenceResolver.cs' 'Core/Resgrid.Chatbot/Handlers/RespondToCallHandler.cs'
ast-grep outline Core/Resgrid.Chatbot/Services/CallReferenceResolver.cs --view expanded || true
ast-grep outline Core/Resgrid.Chatbot/Handlers/RespondToCallHandler.cs --view expanded || true
rg -n "CallReferenceResolver|Resolve.*Call|activeCalls|Shorthand|fire" Core/Resgrid.Chatbot -SRepository: Resgrid/Core
Length of output: 5881
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "hello"Repository: Resgrid/Core
Length of output: 156
🌐 Web query:
"CallReferenceResolver.cs" "RespondToCallHandler" Resgrid.Chatbot
💡 Result:
In the Resgrid Core codebase, CallReferenceResolver.cs and RespondToCallHandler are components associated with the Resgrid Chatbot framework [1][2][3]. The Resgrid Chatbot is a feature of the open-source Resgrid platform—a computer-aided dispatch (CAD) and emergency management system [4]. These specific files are part of the Chatbot's handler architecture, which manages user interactions and automated responses within the system [3][5]. RespondToCallHandler.cs is located within the Core/Resgrid.Chatbot/Handlers/ directory [1][2] and is part of the framework designed to process and respond to dispatch calls via the chatbot interface. CallReferenceResolver.cs is typically involved in the logic that resolves references to specific calls during these chatbot conversations, ensuring the system can correctly identify and perform actions on the relevant CAD call records [5]. These components were introduced as part of the broader development effort (notably associated with task RE1-T117) to implement a functional chatbot framework for the Resgrid platform, enabling personnel to interact with dispatch data through conversational interfaces [3][5].
Citations:
- 1: 9466af3
- 2: a4fa86e
- 3: d54a7b3
- 4: https://github.com/Resgrid/Core
- 5: 9ed7147
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== CallReferenceResolver.cs ==\n'
sed -n '1,140p' Core/Resgrid.Chatbot/Services/CallReferenceResolver.cs
printf '\n== RespondToCallHandler.cs ==\n'
sed -n '1,140p' Core/Resgrid.Chatbot/Handlers/RespondToCallHandler.cs
printf '\n== CallDetailActionHandler.cs ==\n'
sed -n '1,120p' Core/Resgrid.Chatbot/Handlers/CallDetailActionHandler.csRepository: Resgrid/Core
Length of output: 8411
Handle ambiguous shorthand explicitly. fire can match multiple active calls, but this resolver picks the newest one and RespondToCallHandler records the user against it. Return an ambiguity result or require a single match so the handler can ask for a call number.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Resgrid.Chatbot/Services/CallReferenceResolver.cs` around lines 53 - 60,
Update the shorthand resolution logic in CallReferenceResolver to detect when
multiple active calls match the term instead of selecting the newest via
OrderByDescending(...).FirstOrDefault(). Require a single match or return an
explicit ambiguity result that RespondToCallHandler can use to prompt the user
for a call number, while preserving direct resolution for exactly one match.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs (1)
259-282: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicate STOP handling logic into a helper method.
The STOP intent response logic is duplicated identically in three places (here, lines 321-338, and lines 465-482). Consider extracting this into a private helper method to adhere to the DRY principle.
♻️ Proposed refactor
Add a helper method to the class:
private async Task<ChatbotResponse> HandleStopIntentAsync(ChatbotPlatform platform, string userId, ChatbotIntent intent) { if (IsSmsPlatform(platform)) { await _userProfileService.DisableTextMessagesForUserAsync(userId); return new ChatbotResponse { Text = "Text messages are now turned off for this user, to enable again log in to Resgrid and update your profile.", Processed = true, Intent = intent }; } return new ChatbotResponse { Text = "To manage your notification settings, update your profile in Resgrid.", Processed = true, Intent = intent }; }Then replace the three duplicate blocks with calls to this helper. For example, replacing this first block:
- // STOP always works, even in restricted mode — same semantics as the normal-mode - // branch: SMS opts out of all outbound texts; other platforms get the profile guidance. - if (restrictedIntent.Type == ChatbotIntentType.Stop) - { - if (IsSmsPlatform(message.Platform)) - { - await _userProfileService.DisableTextMessagesForUserAsync(identity.UserId); - return new ChatbotResponse - { - Text = "Text messages are now turned off for this user, to enable again log in to Resgrid and update your profile.", - Processed = true, - Intent = restrictedIntent - }; - } - - return new ChatbotResponse - { - Text = "To manage your notification settings, update your profile in Resgrid.", - Processed = true, - Intent = restrictedIntent - }; - } + // STOP always works, even in restricted mode — same semantics as the normal-mode + // branch: SMS opts out of all outbound texts; other platforms get the profile guidance. + if (restrictedIntent.Type == ChatbotIntentType.Stop) + { + return await HandleStopIntentAsync(message.Platform, identity.UserId, restrictedIntent); + }And similarly for the blocks at line 321 (
new ChatbotIntent { Type = ChatbotIntentType.Stop, Confidence = 1.0f }) and line 465 (intent).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs` around lines 259 - 282, Extract the duplicated STOP response logic into a private async helper near the chatbot handling methods, accepting the platform, user ID, and intent and preserving the SMS opt-out behavior and platform-specific response text. Replace all three STOP handling blocks, including the branches using constructed and existing intents, with calls to this helper while preserving their current return behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Core/Resgrid.Chatbot.NLU/Providers/KeywordIntentClassifier.cs`:
- Around line 93-94: Update the call-detail pattern and mapping in the keyword
intent definitions so full call references such as “c26-1” are matched before
the raw numeric call-ID form. Return the complete matched reference under
callRef for hyphenated values, while preserving callId extraction for plain
numeric references; anchor the change to the call_detail rule and its capture
mapping.
In `@Core/Resgrid.Chatbot/Handlers/DepartmentActionHandler.cs`:
- Around line 199-200: Validate trimmedId after normalization in the
department-switch handling flow and reject or return without switching when it
is empty. Ensure the later department lookup using IndexOf only runs for
non-empty identifiers, preserving existing behavior for valid identifiers.
---
Nitpick comments:
In `@Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs`:
- Around line 259-282: Extract the duplicated STOP response logic into a private
async helper near the chatbot handling methods, accepting the platform, user ID,
and intent and preserving the SMS opt-out behavior and platform-specific
response text. Replace all three STOP handling blocks, including the branches
using constructed and existing intents, with calls to this helper while
preserving their current return behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 79f15ef4-7e3e-470c-8306-eca590a97470
📒 Files selected for processing (8)
Core/Resgrid.Chatbot.NLU/Providers/KeywordIntentClassifier.csCore/Resgrid.Chatbot/Handlers/DepartmentActionHandler.csCore/Resgrid.Chatbot/Handlers/HelpActionHandler.csCore/Resgrid.Chatbot/Handlers/StaffingActionHandler.csCore/Resgrid.Chatbot/Handlers/StatusActionHandler.csCore/Resgrid.Chatbot/Services/CallReferenceResolver.csCore/Resgrid.Chatbot/Services/ChatbotIngressService.csWorkers/Resgrid.Workers.Framework/Logic/ChatbotMessageLogic.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- Core/Resgrid.Chatbot/Services/CallReferenceResolver.cs
- Core/Resgrid.Chatbot/Handlers/HelpActionHandler.cs
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| (R(@"^(responding|1)$"), "set_status", m => P("actionType", "2")), | ||
| (R(@"^(not\s*responding|2)$"), "set_status", m => P("actionType", "1")), | ||
| (R(@"^(on\s*scene|onscene|3)$"), "set_status", m => P("actionType", "3")), | ||
| (R(@"^(standing\s*by|standingby|4)$"), "set_status", m => P("actionType", "0")), |
There was a problem hiding this comment.
Duplicated string literals: intent names such as "set_status", "list_calls", "call_detail", and "set_staffing" are defined as inline strings in KeywordIntentClassifier.cs and repeated verbatim in IntentMapper.cs and ChatbotTemplateRenderer.cs. Centralize these shared keys as constants in a single location (e.g., an IntentNames class) to prevent silent mismatches when one site is edited independently.
Kody rule violation: Centralize string constants
// Define intent name constants once, e.g. in a shared IntentNames class:
// public static class IntentNames { public const string SetStatus = "set_status"; ... }
// Then reference IntentNames.SetStatus here and in IntentMapper.cs / ChatbotTemplateRenderer.cs.
(R(@"^(responding|1)$"), IntentNames.SetStatus, m => P("actionType", ((int)ActionTypes.Responding).ToString())),Prompt for LLM
File Core/Resgrid.Chatbot.NLU/Providers/KeywordIntentClassifier.cs:
Line 21 to 24:
Intent name string literals ("set_status", "list_calls", "call_detail", "set_staffing", etc.) are defined as inline strings in KeywordIntentClassifier.cs and duplicated verbatim in IntentMapper.cs and ChatbotTemplateRenderer.cs (verified via grep). These shared keys should be centralized as constants to prevent a silent mismatch if one location is edited but not the others. Rule 7 requires defining shared string literals as constants/enums in a single place.
Suggested Code:
// Define intent name constants once, e.g. in a shared IntentNames class:
// public static class IntentNames { public const string SetStatus = "set_status"; ... }
// Then reference IntentNames.SetStatus here and in IntentMapper.cs / ChatbotTemplateRenderer.cs.
(R(@"^(responding|1)$"), IntentNames.SetStatus, m => P("actionType", ((int)ActionTypes.Responding).ToString())),
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| (R(@"^(responding|1)$"), "set_status", m => P("actionType", "2")), | ||
| (R(@"^(not\s*responding|2)$"), "set_status", m => P("actionType", "1")), | ||
| (R(@"^(on\s*scene|onscene|3)$"), "set_status", m => P("actionType", "3")), | ||
| (R(@"^(standing\s*by|standingby|4)$"), "set_status", m => P("actionType", "0")), |
There was a problem hiding this comment.
Magic strings: actionType values use hardcoded string representations of ActionTypes enum values ("0"=StandingBy, "1"=NotResponding, "2"=Responding, "3"=OnScene) and staffingType values mirror UserStateTypes ("0"-"4"). If either enum is reordered, these literals will silently produce incorrect statuses; cast the enum constants directly via ((int)ActionTypes.Responding).ToString() to eliminate the risk.
Kody rule violation: Replace magic numbers with named constants
(R(@"^(responding|1)$"), "set_status", m => P("actionType", ((int)ActionTypes.Responding).ToString())),
(R(@"^(not\s*responding|2)$"), "set_status", m => P("actionType", ((int)ActionTypes.NotResponding).ToString())),
(R(@"^(on\s*scene|onscene|3)$"), "set_status", m => P("actionType", ((int)ActionTypes.OnScene).ToString())),
(R(@"^(standing\s*by|standingby|4)$"), "set_status", m => P("actionType", ((int)ActionTypes.StandingBy).ToString())),Prompt for LLM
File Core/Resgrid.Chatbot.NLU/Providers/KeywordIntentClassifier.cs:
Line 21 to 24:
The actionType parameter values use hardcoded string representations of ActionTypes enum values ("0"=StandingBy, "1"=NotResponding, "2"=Responding, "3"=OnScene) and staffingType values mirror UserStateTypes ("0"-"4"). The comments at lines 19-20 and 34-35 acknowledge these are enum values, yet the enum constants are not used. If the ActionTypes or UserStateTypes enum values are ever reordered, these string literals will silently produce incorrect statuses. Rule 10 requires extracting numeric literals with meaning into named constants; the enum itself provides the canonical constant.
Suggested Code:
(R(@"^(responding|1)$"), "set_status", m => P("actionType", ((int)ActionTypes.Responding).ToString())),
(R(@"^(not\s*responding|2)$"), "set_status", m => P("actionType", ((int)ActionTypes.NotResponding).ToString())),
(R(@"^(on\s*scene|onscene|3)$"), "set_status", m => P("actionType", ((int)ActionTypes.OnScene).ToString())),
(R(@"^(standing\s*by|standingby|4)$"), "set_status", m => P("actionType", ((int)ActionTypes.StandingBy).ToString())),
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| "notresponding" => (int)Model.ActionTypes.NotResponding, | ||
| "onscene" => (int)Model.ActionTypes.OnScene, | ||
| "standingby" => (int)Model.ActionTypes.StandingBy, | ||
| "available" => (int)Model.ActionTypes.AvailableStation, |
There was a problem hiding this comment.
Inconsistent status resolution: the word "available" maps to ActionTypes.AvailableStation in ResolveStatusNameAsync but resolves to StandingBy (0) via MapStatusWord, causing equivalent natural-language inputs to set different statuses and produce inconsistent confirmation messages. Map "available" to ActionTypes.StandingBy in ResolveStatusNameAsync's standard-words switch to match MapStatusWord and the documented scheme.
"available" => (int)Model.ActionTypes.StandingBy,Prompt for LLM
File Core/Resgrid.Chatbot/Handlers/StatusActionHandler.cs:
Line 96:
WHAT: The word "available" resolves to two different ActionTypes depending on phrasing: "I'm available" → MapStatusWord → "0" (StandingBy), but "SET STATUS TO available" → ResolveStatusNameAsync → ActionTypes.AvailableStation (4). WHY: Equivalent natural-language inputs set different statuses — StandingBy(0, displays "Available") vs AvailableStation(4, displays "Available Station") — producing inconsistent confirmation messages and, in dispatch contexts, different station-presence routing semantics; this is the only standard status word where the two NEW paths diverge, and it contradicts the author's documented intent ("StandingBy/Available=0"). HOW: In ResolveStatusNameAsync's standard-words switch, map "available" to ActionTypes.StandingBy to match MapStatusWord and the documented scheme.
Suggested Code:
"available" => (int)Model.ActionTypes.StandingBy,
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var customStates = await _customStateService.GetAllActiveCustomStatesForDepartmentAsync(departmentId); | ||
| var customActions = customStates?.FirstOrDefault(x => x.Type == (int)Model.CustomStateTypes.Personnel); | ||
| if (customActions != null && !customActions.IsDeleted && customActions.GetActiveDetails()?.Any() == true) | ||
| { | ||
| var detail = customActions.GetActiveDetails() | ||
| .FirstOrDefault(d => string.Equals(d.ButtonText?.Trim(), name, StringComparison.OrdinalIgnoreCase) | ||
| || string.Equals(d.ButtonText?.Replace(" ", ""), name.Replace(" ", ""), StringComparison.OrdinalIgnoreCase)); | ||
|
|
||
| if (detail != null) | ||
| return detail.CustomStateDetailId; |
There was a problem hiding this comment.
Redundant allocation: GetActiveDetails() is invoked twice in ResolveStatusNameAsync — first in the guard at line 79 and again at line 81 — where each call allocates a new filtered and sorted List (CustomState.cs:44-50). The same pattern appears in ResolveStaffingNameAsync (StaffingActionHandler.cs lines 78/80); compute the result once into a local variable and reuse it, consistent with the existing convention in TwilioController.cs:396.
Kody rule violation: Use computed/derived properties for repeated calculations
var activeDetails = customActions?.GetActiveDetails();
if (customActions != null && !customActions.IsDeleted && activeDetails?.Any() == true)
{
var detail = activeDetails
.FirstOrDefault(d => ...);Prompt for LLM
File Core/Resgrid.Chatbot/Handlers/StatusActionHandler.cs:
Line 77 to 86:
Violates rule 'Use computed/derived properties for repeated calculations': GetActiveDetails() is called twice in the new ResolveStatusNameAsync method — first in the guard `customActions.GetActiveDetails()?.Any() == true` (line 79) and again immediately after in `customActions.GetActiveDetails().FirstOrDefault(...)` (line 81). GetActiveDetails() allocates a new filtered+sorted List each invocation (CustomState.cs:44-50 does Details.Where(...).OrderBy(...).ToList()). The same pattern appears in the new ResolveStaffingNameAsync (StaffingActionHandler.cs lines 78/80). The existing codebase convention caches this value into a local variable before reuse (e.g., TwilioController.cs:396). Compute it once and reuse the result.
Suggested Code:
var activeDetails = customActions?.GetActiveDetails();
if (customActions != null && !customActions.IsDeleted && activeDetails?.Any() == true)
{
var detail = activeDetails
.FirstOrDefault(d => ...);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| @@ -282,6 +306,37 @@ await _userIdentityService.LinkUserAsync( | |||
| return new ChatbotResponse { Text = optionsBuilder.ToString(), Processed = true }; | |||
There was a problem hiding this comment.
Duplicated business logic: the STOP telecom-opt-out sequence — IsSmsPlatform check, DisableTextMessagesForUserAsync call, and hardcoded response return — is copy-pasted across three branches in ChatbotIngressService.cs (lines 262-281, 316-338, and 463-482), varying only in the attached ChatbotIntent. Extract a single private helper such as HandleStopOptOutAsync(string userId, ChatbotPlatform platform, ChatbotIntent intent) and call it from all three sites so opt-out semantics and response text live in one place.
Kody rule violation: Extract duplicated business logic
private async Task<ChatbotResponse> HandleStopOptOutAsync(string userId, ChatbotPlatform platform, ChatbotIntent intent)
{
if (IsSmsPlatform(platform))
{
await _userProfileService.DisableTextMessagesForUserAsync(userId);
return new ChatbotResponse
{
Text = "Text messages are now turned off for this user, to enable again log in to Resgrid and update your profile.",
Processed = true,
Intent = intent
};
}
return new ChatbotResponse
{
Text = "To manage your notification settings, update your profile in Resgrid.",
Processed = true,
Intent = intent
};
}
// Each call site reduces to:
return await HandleStopOptOutAsync(identity.UserId, message.Platform, restrictedIntent);Prompt for LLM
File Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs:
Line 306:
Violates rule 'Extract duplicated business logic': the STOP telecom-opt-out logic is copy-pasted three times in this file — the restricted-mode branch (lines 262-281), the new verbatim STOP check (lines 316-338), and the cloud-NLU STOP branch (lines 463-482). All three blocks perform the identical sequence: check IsSmsPlatform, call DisableTextMessagesForUserAsync(identity.UserId), and return one of two hardcoded response strings. The only variation is the ChatbotIntent variable attached to the response. Extract a single private helper such as `HandleStopOptOutAsync(string userId, ChatbotPlatform platform, ChatbotIntent intent)` and call it from all three sites, so the opt-out semantics and response text live in one place.
Suggested Code:
private async Task<ChatbotResponse> HandleStopOptOutAsync(string userId, ChatbotPlatform platform, ChatbotIntent intent)
{
if (IsSmsPlatform(platform))
{
await _userProfileService.DisableTextMessagesForUserAsync(userId);
return new ChatbotResponse
{
Text = "Text messages are now turned off for this user, to enable again log in to Resgrid and update your profile.",
Processed = true,
Intent = intent
};
}
return new ChatbotResponse
{
Text = "To manage your notification settings, update your profile in Resgrid.",
Processed = true,
Intent = intent
};
}
// Each call site reduces to:
return await HandleStopOptOutAsync(identity.UserId, message.Platform, restrictedIntent);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
2 participants
This pull request significantly enhances the Resgrid Chatbot's natural language understanding, help system, and SMS interaction model, making it more intuitive for first responders who interact primarily via text messages.
Key Changes
Smarter Intent Recognition (NLU)
Flexible Call Reference Resolution
CallReferenceResolverthat resolves references by raw ID, call number (year-sequence format), or shorthand term (e.g., "fire") matched against active calls' name, type, and nature. Department-level isolation (anti-IDOR) is enforced throughout.Two-Level Help System
HelpActionHandlerso bare "HELP" returns a concise topic menu (to minimize SMS segments/costs), and "HELP " returns detailed commands for that specific area. Status and staffing topics reflect each department's custom states.STOP as Telecom Opt-Out
Interactive SMS Length Cap
SmsReplyMaxLength(1500 chars) for chatbot replies, which are interactive content users act on over SMS — higher than notification defaults but within carrier/Twilio delivery limits.Updated Tests & Localization
Call_NoMatchresources across all supported languages.Summary by CodeRabbit