feat(transfer_to_human): guarantee the customer notice before transferring - #57
Open
mateusbellozupko wants to merge 4 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 #1 — e.g. an "Imposto de Renda" request got sent to "Dep. Contábil/Fiscal" (rule #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 GuideThe PR makes transfer notifications reliable by sending an explicit customer message before assignment and exposing delivery failures without blocking routing. It also changes configured-rule handling to favor an explicit model-selected rule, adds safer keyword-based selection when omitted, and documents the required rule and notification parameters. Sequence diagram for customer notice before human transfersequenceDiagram
participant Agent as LLM agent
participant Tool as transfer_to_human
participant CRM as CRM API
Agent->>Tool: transfer_to_human(rule_index, message_to_customer)
Tool->>CRM: POST /conversations/{id}/messages\nprivate=false
alt notice sent
CRM-->>Tool: message accepted
else notice fails
CRM-->>Tool: error
end
Tool->>CRM: Assign conversation to agent or team
CRM-->>Tool: transfer result
Tool-->>Agent: status + message_to_customer_sent\nmessage_to_customer_error
Flow diagram for configured transfer-rule selectionflowchart TD
A[transfer_to_human called] --> B{rule_index provided?}
B -->|Yes| C[Select configured rule by index]
B -->|No| D{reason matches rule instructions?}
D -->|Yes| E[Select highest keyword-overlap rule]
D -->|No| F[Fall back to first valid configured rule]
C --> G[Resolve agent or team destination]
E --> G
F --> G
G --> H[Send customer notice if provided]
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
Author
|
@sourcery-ai review — both findings are already addressed on this branch: an out-of-range/zero |
There was a problem hiding this comment.
Hey - I've found 1 issue
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="274-279" />
<code_context>
)
-
+
+ # Send the customer-facing notice BEFORE assigning, so it doesn't
+ # depend on the model also producing text in the same turn (it
+ # often doesn't when it's already decided to call tools) — every
+ # transfer rule's own instructions say to notify the customer
+ # first, so make that guaranteed rather than hoped-for (EVO-2249).
+ message_send_error = None
+ if message_to_customer and message_to_customer.strip():
+ formatted_message = message_to_customer.strip()
</code_context>
<issue_to_address>
**issue (broader_impact):** The customer notice is committed in a separate POST before the assignment POST, so if assignment fails after the notice succeeds, the customer receives a message promising an imminent transfer even though no transfer occurred. The assignment-error response also omits `message_to_customer_sent` and `message_to_customer_error`, so a caller or model retry cannot tell that the notice was already sent and can send it again.
**Triggers:** When the customer-message POST succeeds but the subsequent assignment POST fails.
**Suggested fix:** Include the notice outcome in every return path and make retries idempotent, or use an API-level transaction/rollback where available; at minimum, avoid presenting the notice as a completed transfer when assignment fails.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the rule selection or generated notice is wrong, the change can route the conversation to the wrong agent or team and send an incorrect customer-facing message before the transfer. Reverting prevents future occurrences but cannot retract messages already sent or undo transfers that already happened.
Blocking findings: src/services/adk/tools/evo_crm/transfer_to_human.py:279
Author
|
Fixed — |
…ure too The customer notice and the assignment are two separate, already-committed POSTs. message_to_customer_sent/message_to_customer_error were only included in the success response — if the notice succeeded but the subsequent assignment POST failed, the error response gave no indication the customer had already been told a transfer was imminent, so a caller or model retry had no way to know not to resend the same notice. Computed once right after the notice attempt so every return path (success, assignment error, and the outer unexpected-error handler) reports the same outcome. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
Ensure transfer actions select the appropriate configured rule and notify the customer before routing whenever required.
New Features:
Bug Fixes:
Enhancements:
Tests: