fix(transfer_to_human): stop always routing to the first configured rule - #55
Open
mateusbellozupko wants to merge 2 commits into
Open
Conversation
When the model called transfer_to_human with only `reason` (no explicit team_id/assignee_id — the common case, since it only knows team NAMES from the docstring, not their opaque IDs), the tool silently picked whichever transfer rule happened to be first in the configured list, regardless of the actual reason. Every escalation for an agent with multiple transfer rules was routed to rule evolution-foundation#1 — e.g. an "Imposto de Renda" request got sent to "Dep. Contábil/Fiscal" (rule evolution-foundation#1) instead of the dedicated IRPF team (rule evolution-foundation#4), because the code never evaluated rule conditions at all (the old comment literally said "In the future, this could be enhanced to evaluate rule conditions"). Fix: - Add a `rule_index` parameter and instruct the model (via the numbered rule list already in its docstring) to always pass it when transfer_rules are configured. - If rule_index is omitted, fall back to matching `reason` against each rule's own `instructions` text by keyword overlap. - Only if neither yields a match, fall back to the first configured rule (previous behavior), now logged as a warning so misroutes are visible. Reproduced live: an "Imposto de Renda Pessoa Física" request was transferred to "Dep. Contábil/Fiscal" instead of the dedicated team.
Reviewer's GuideTransfer routing now prioritizes the model-selected configured rule, with reason-based matching as a secondary path and an explicitly logged first-rule fallback; the tool documentation is updated to guide models toward passing the new rule_index. Sequence diagram for model-guided transfer routingsequenceDiagram
participant Model
participant Tool as transfer_to_human
participant Rules as ConfiguredTransferRules
participant CRM
Model->>Tool: transfer_to_human(rule_index, reason)
Tool->>Rules: Select configured rule by rule_index
Rules-->>Tool: Selected rule
Tool->>Tool: Resolve userId or teamId
Tool->>CRM: Transfer conversation to selected destination
CRM-->>Tool: Transfer result
Tool-->>Model: Transfer status
Flow diagram for transfer rule selectionflowchart TD
A[transfer_to_human called] --> B{Explicit assignee_id or team_id?}
B -->|Yes| Z[Use explicit destination]
B -->|No| C{rule_index selects a configured rule?}
C -->|Yes| D[Use selected rule]
C -->|No| E{reason matches rule instructions?}
E -->|Yes| D
E -->|No| F[Use first valid configured rule]
F --> G[Log warning about possible misrouting]
D --> H[Resolve userId or teamId]
G --> H
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/services/adk/tools/evo_crm/transfer_to_human.py" line_range="353-357" />
<code_context>
- you must provide assignee_id or team_id.{transfer_rules_doc}
-
+
+ If transfer_rules are configured, you MUST pass rule_index set to the number of
+ whichever configured rule below actually matches what the user asked about — do
+ not omit it and rely on a default, there is no single "default" rule and omitting
+ it risks the wrong team. Otherwise, provide assignee_id or team_id
+ explicitly.{transfer_rules_doc}
+
Args:
</code_context>
<issue_to_address>
**issue (broader_impact):** The new tool documentation requires the model to pass `rule_index`, but the agent-level CRM prompt still tells the model that configured rules are applied automatically and that it does not need to provide assignment parameters. Models receiving both instructions continue omitting `rule_index`; when the reason has no keyword overlap with the rules, the implementation still routes to the first valid rule.
**Triggers:** When the tool is used through `LlmAgentBuilder` with configured transfer rules and the reason does not contain a token found in any rule's instructions.
**Suggested fix:** Update the agent-level prompt to require the numbered `rule_index` and remove the statement that configured rules are selected automatically.
</issue_to_address>
### Comment 2
<location path="src/services/adk/tools/evo_crm/transfer_to_human.py" line_range="176-181" />
<code_context>
+ reason_lower = reason.lower()
+ for rule in available_transfer_rules:
+ instructions = (rule.get("instructions") or "").lower()
+ if instructions and any(
+ word in instructions
+ for word in reason_lower.split()
+ if len(word) > 3
+ ):
+ selected_rule = rule
+ logger.info(
+ "Matched transfer rule by keyword overlap between "
</code_context>
<issue_to_address>
**issue (bug_risk):** The keyword matcher treats any reason substring longer than three characters as a match, without token boundaries, stop-word filtering, or relevance scoring. A generic word such as `need`, `para`, or `help` appearing in an earlier rule's instructions selects that rule before a later rule with the actual topic match, causing another misroute.
**Triggers:** When a reason contains a common word that appears in an earlier rule's instructions and the intended rule appears later.
**Suggested fix:** Normalize and tokenize both texts, remove stop words, and require meaningful word or phrase overlap; prefer the rule with the strongest match rather than stopping at the first match.
</issue_to_address>
### Comment 3
<location path="src/services/adk/tools/evo_crm/transfer_to_human.py" line_range="162-189" />
<code_context>
+ if rule_index is not None and 1 <= rule_index <= len(available_transfer_rules):
</code_context>
<issue_to_address>
**issue (bug_risk):** An out-of-range or otherwise invalid `rule_index` is silently discarded and the code proceeds to keyword matching or the first-rule fallback. A caller that explicitly selected a nonexistent rule therefore receives a successful transfer to a different rule instead of an invalid-argument error, hiding model/tool contract failures and potentially misrouting the conversation.
**Triggers:** When the model or caller supplies a `rule_index` less than 1 or greater than the configured rule count.
**Suggested fix:** Validate a non-null `rule_index` and return an error when it is outside the configured 1-based range instead of treating it as omitted.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 3 findings to address first, and a bad rule index or keyword match can assign a conversation to the wrong human or team, and that assignment may persist after the code is reverted. The conversation can be reassigned or the job rerun to repair it, but any incorrect recipient access or handling that occurred before correction would not be undone.
Blocking findings: src/services/adk/tools/evo_crm/transfer_to_human.py:357, src/services/adk/tools/evo_crm/transfer_to_human.py:181, src/services/adk/tools/evo_crm/transfer_to_human.py:189
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+353
to
+357
| If transfer_rules are configured, you MUST pass rule_index set to the number of | ||
| whichever configured rule below actually matches what the user asked about — do | ||
| not omit it and rely on a default, there is no single "default" rule and omitting | ||
| it risks the wrong team. Otherwise, provide assignee_id or team_id | ||
| explicitly.{transfer_rules_doc} |
There was a problem hiding this comment.
issue (broader_impact): The new tool documentation requires the model to pass rule_index, but the agent-level CRM prompt still tells the model that configured rules are applied automatically and that it does not need to provide assignment parameters. Models receiving both instructions continue omitting rule_index; when the reason has no keyword overlap with the rules, the implementation still routes to the first valid rule.
Triggers: When the tool is used through LlmAgentBuilder with configured transfer rules and the reason does not contain a token found in any rule's instructions.
Suggested fix: Update the agent-level prompt to require the numbered rule_index and remove the statement that configured rules are selected automatically.
- Explicit but out-of-range rule_index now returns an error instead of silently falling through to keyword matching or the first rule. - Keyword fallback scores rules by count of shared meaningful, non-stopword tokens and picks the best match instead of stopping at the first rule containing any word longer than 3 chars. - The agent-level system prompt said "the tool will automatically use the configured transfer rules, so you don't need to specify assignee_id or team_id" — directly contradicting the rule_index requirement. Now numbers the rules and explicitly requires rule_index.
Author
|
Fixed in 5f1e8e0 — invalid rule_index now returns an explicit error instead of silently falling through; keyword fallback now scores rules by shared non-stopword tokens instead of stopping at the first word match; and the agent-level prompt (llm_agent_builder.py) no longer contradicts the rule_index requirement by claiming rules apply automatically. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the model calls
transfer_to_humanwith onlyreason(no explicitteam_id/assignee_id— the common case, since the model only knows team names from the tool's docstring, not their opaque IDs), the tool silently picked whichever transfer rule happened to be listed first, completely ignoring the actual reason. The old code's own comment admitted this: "Use the first transfer rule that matches 'human' or 'team' ... In the future, this could be enhanced to evaluate rule conditions."Practical effect: any agent with more than one transfer rule always routed every escalation to rule #1, regardless of topic.
Fix
rule_indexparameter, and instruct the model (via the numbered rule list already built into its docstring) to always pass it when transfer_rules are configured.rule_indexis omitted, fall back to matchingreasonagainst each rule's owninstructionstext by keyword overlap.Testing notes
Reproduced live against a real agent with 8 transfer rules: a user message about "Imposto de Renda Pessoa Física" (rule #4) was being routed to "Dep. Contábil/Fiscal" (rule #1) every time. Confirmed via the tool's own JSON-RPC response, which showed
team_idresolving to rule #1's team regardless of the stated reason.🤖 Generated with Claude Code
Summary by Sourcery
Route human transfers to the rule that best matches the escalation instead of silently defaulting to the first configured rule.
New Features:
Bug Fixes:
Enhancements: