fix(dispatch): stop prepending the agent-bot signature (double-apply) - #11
Open
mateusbellozupko wants to merge 2 commits into
Open
mateusbellozupko wants to merge 2 commits into
mateusbellozupko wants to merge 2 commits into
Conversation
dispatch_engine.go prepended cfg.MessageSignature to the first message
part with no separator, so a signature like "Atendente" glued straight
onto the reply text ("AtendentePronto, Mateus!..."). The CRM now
applies this prefix itself, once, for every outgoing AgentBot message
regardless of which path created it (Message#apply_agent_bot_signature
in evo-ai-crm-community) — including this dispatcher's postback.
Prepending it here too would double it.
BotConfig.MessageSignature is left on the struct for callers that
still read it; Dispatch itself no longer acts on it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reviewer's GuideThis PR prevents double application of the agent-bot display-name prefix by making bot_runtime leave dispatch content untouched; tests now validate that behavior across unit and end-to-end segmentation paths. Sequence diagram for centralized AgentBot signature applicationsequenceDiagram
participant Dispatch as bot_runtime Dispatch
participant CRM as CRM
participant AgentBot as AgentBot recipient
Dispatch->>Dispatch: segmentContent(residual, cfg)
Dispatch->>CRM: Send dispatch parts without MessageSignature
CRM->>CRM: Apply display-name prefix once
CRM->>AgentBot: Deliver prefixed message
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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="pkg/dispatch/service/dispatch_engine.go" line_range="84" />
<code_context>
- if cfg.MessageSignature != "" && len(parts) > 0 {
- parts[0] = cfg.MessageSignature + parts[0]
- }
+ // FR-21's agent-bot display-name prefix is no longer applied here: the CRM
+ // now applies it once, centrally, for every outgoing AgentBot message
+ // (including this postback), so doing it here too would double-prefix it.
</code_context>
<issue_to_address>
**nitpick:** The `DispatchEngine` interface comment still says dispatch appends the message signature, but `Dispatch` now deliberately sends content unchanged, so the public contract documentation is false.
**Suggested fix:** Update the interface comment to state that dispatch segments and sends the AI response without applying `MessageSignature`.
</issue_to_address>
### Comment 2
<location path="pkg/dispatch/service/dispatch_engine_test.go" line_range="75-77" />
<code_context>
- }
- for i, p := range parts[1:] {
+ for i, p := range parts {
if strings.Contains(p, "[bot]") {
- t.Errorf("signature must NOT be on part %d: %q", i+1, p)
+ t.Errorf("MessageSignature must not be applied by bot_runtime, found it on part %d: %q", i, p)
}
}
}
</code_context>
<issue_to_address>
**nitpick (testing):** The multipart regression test only checks that `[bot]` is absent; it passes if dispatch drops, alters, or sends the wrong segmented content as long as that marker is missing, so it does not verify the stated requirement that message content remains untouched.
**Triggers:** When a future change regresses segmentation or modifies content without adding the signature marker.
**Suggested fix:** Assert the complete expected parts, such as `[]string{"hello world this", "is test"}`, in addition to checking that the signature is absent.
</issue_to_address>Sourcery assessment
Needs a human reviewer. If the CRM does not apply the prefix exactly as assumed, outgoing AgentBot messages will be sent with a missing or double signature. Reverting restores the old behavior for future messages, but it cannot undo messages already delivered externally.
… test - DispatchEngine's interface comment still said it appends the message signature; it deliberately doesn't since the CRM prefix centralization. - The multipart test only checked "[bot]" was absent, which would still pass if a regression dropped or mangled segment content as long as that literal string was missing. Assert the exact expected parts. 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
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
dispatch_engine.goprependedcfg.MessageSignatureto the first message part with no separator (cfg.MessageSignature + parts[0]), so a signature like"Atendente"glued straight onto the reply text ("AtendentePronto, Mateus!...").BotConfig.MessageSignaturestays on the struct for callers that still read it, butDispatchno longer acts on it.Test plan
dispatch_engine_test.goandtest/e2e/e2e_test.goto assert the signature is not applied by bot_runtime.go test ./...locally in this environment (no Go toolchain available) — relying on CI.Summary by Sourcery
Remove bot_runtime signature application so the CRM can add the agent-bot prefix exactly once to outgoing messages.
Bug Fixes:
Enhancements:
Tests: