feat(transfer_to_human): guarantee the customer notice before transferring - #57
Open
mateusbellozupko wants to merge 3 commits into
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.
…rring Every transfer rule's own instructions ask the agent to notify the customer before transferring, but that depended entirely on the model also producing reply text in the same turn as its tool calls — and when the model decides to call tools, it frequently skips the customer-facing text entirely (observed live: manage_conversation_labels + transfer_to_human fired together with a null/empty assistant message, so the transfer happened completely silently). Add a message_to_customer parameter: when provided, the tool sends it as a real outgoing message BEFORE performing the team/agent assignment, so "notify then transfer" is one atomic tool action instead of two separate things the model has to remember to do in sequence. Docstring now instructs the model to always pass it when the matched rule's instructions ask for a customer notice.
Reviewer's GuideUpdates human transfer handling so the tool can reliably send a customer-facing notice before assignment, surfaces notice delivery failures without blocking routing, and guides the model toward explicit, rule-aware destination selection. Sequence diagram for customer notice before human transfersequenceDiagram
participant Model
participant Tool as transfer_to_human
participant CRM as CRM API
Model->>Tool: transfer_to_human(message_to_customer, rule_index)
Tool->>Tool: Select transfer rule
opt message_to_customer provided
Tool->>CRM: POST /conversations/{id}/messages(private: false)
alt Notice sent
CRM-->>Tool: Success
else Notice fails
CRM-->>Tool: Error
Tool->>Tool: Record message_to_customer_error
end
end
Tool->>CRM: Assign conversation to agent or team
CRM-->>Tool: Transfer result
Tool-->>Model: status, message_to_customer_sent, message_to_customer_error
Flow diagram for rule-aware transfer selectionflowchart TD
A[Configured transfer rules] --> B{Valid rule_index provided?}
B -->|Yes| C[Select indexed rule]
B -->|No| D{Reason matches rule instructions?}
D -->|Yes| E[Select matching rule]
D -->|No| F[Fall back to first valid rule]
C --> G[Resolve agent or team destination]
E --> G
F --> G
G --> H[Send customer notice]
H --> I[Assign conversation]
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 2 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="177-180" />
<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
</code_context>
<issue_to_address>
**issue (bug_risk):** The fallback matcher treats any substring overlap with any reason word longer than three characters as a rule match, so common words such as “para”, “cliente”, or “humano” select the first rule containing that word even when the rule is unrelated to the escalation. This misroutes transfers whenever `rule_index` is omitted and multiple rules share ordinary instruction wording.
**Triggers:** When the model omits `rule_index` and the reason contains a common word present in an earlier rule's instructions.
**Suggested fix:** Tokenize and normalize meaningful keywords or use an explicit ambiguity check instead of accepting the first arbitrary substring overlap.
</issue_to_address>
### Comment 2
<location path="src/services/adk/tools/evo_crm/transfer_to_human.py" line_range="163-165" />
<code_context>
+
+ # Preferred path: the model picked a specific rule from the
+ # numbered list in its own docstring (see transfer_rules_doc).
+ if rule_index is not None and 1 <= rule_index <= len(available_transfer_rules):
+ selected_rule = available_transfer_rules[rule_index - 1]
+ logger.info(f"Using transfer rule #{rule_index} selected by the model")
+
+ # No explicit index: previously this silently fell back to
</code_context>
<issue_to_address>
**issue (bug_risk):** An out-of-range or zero `rule_index` is silently ignored and the code proceeds to reason matching or the first valid configured rule, so an invalid model selection still performs a potentially incorrect transfer instead of reporting that the requested rule does not exist.
**Triggers:** When the model supplies `rule_index` as 0 or a number greater than the configured rule count.
**Suggested fix:** Validate any non-`None` `rule_index` and return an error before selecting a fallback rule when it is outside the configured range.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and the change can send a customer-facing message before the transfer, and an incorrect or unintended notice cannot be fully undone by reverting the code. Its rule-selection changes can also route a conversation to the wrong configured team or agent, although the resulting transfer is otherwise bounded and can be corrected manually.
Blocking findings: src/services/adk/tools/evo_crm/transfer_to_human.py:180, src/services/adk/tools/evo_crm/transfer_to_human.py:165
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- 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 + message_to_customer.
Author
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
Every transfer rule's own
instructionsfield asks the agent to notify the customer before transferring, but that depended entirely on the model also producing reply text in the same turn as its tool calls. In practice, when a model decides to call tools it frequently omits customer-facing text — observed live:manage_conversation_labels+transfer_to_humanfired together with a null/empty assistant message, so the transfer happened completely silently, with no "vou te encaminhar..." notice ever sent.Fix
Add a
message_to_customerparameter totransfer_to_human. When provided, the tool sends it as a real outgoing message before performing the team/agent assignment — so "notify, then transfer" becomes one atomic tool action instead of two separate things the model has to remember to sequence correctly. The docstring now instructs the model to always pass this when the matched rule's instructions ask for a customer notice. The response includesmessage_to_customer_sent/message_to_customer_errorso a failed notice doesn't silently disappear (and doesn't block the transfer itself — better routed without the notice than not routed at all).Stacked on
This branches from #55 (
rule_index), since the docstring and rule-selection logic it touches were added there.Testing notes
Verified the message-send call reuses the same
POST /conversations/{id}/messagespattern as the existingsend_private_messagetool, just withprivate: falsefor a customer-visible outgoing message.🤖 Generated with Claude Code
Summary by Sourcery
Guarantee customer notification before transfers and improve selection of the configured destination rule.
New Features:
Bug Fixes:
Enhancements: