Skip to content
This repository was archived by the owner on Apr 12, 2026. It is now read-only.
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions internal/slack/agentCallbackHandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ package slackbot

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add unit tests for the new functions? Specifically:

  • isThinkingMessage() with positive/negative cases
  • formatFinalResponse() verifying prefix removal
  • formatContextMessageBlock() verifying JSON output structure

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Added internal/slack/agentCallbackHandler_test.go covering isThinkingMessage, formatFinalResponse, and verifying the JSON produced by formatContextMessageBlock.

import (
"context"
"encoding/json"
"github.com/tmc/langchaingo/callbacks"
"regexp"

"github.com/slack-go/slack"
)

type sendMessageFunc func(message string)
Expand All @@ -15,7 +19,49 @@ type agentCallbackHandler struct {
func (handler *agentCallbackHandler) HandleChainEnd(_ context.Context, outputs map[string]any) {
if text, ok := outputs["text"]; ok {
if textStr, ok := text.(string); ok {
if isThinkingMessage(textStr) {
textStr = formatContextMessageBlock(textStr)
} else {
textStr = formatFinalResponse(textStr)
}
handler.sendMessage(textStr)
}
}
}

var (
thinkingPattern = regexp.MustCompile(`Do I need to use a tool\? Yes`)
cleanupPattern = regexp.MustCompile(`(Do I need to use a tool\? No|AI:)`)
)

func isThinkingMessage(msg string) bool {
return thinkingPattern.MatchString(msg)
}

// formatFinalResponse removes LLM agent response prefixes.
// The agent response format is defined in internal/llm/langchain.go
// > Thought: Do I need to use a tool? No
// > AI: [your response here]
func formatFinalResponse(msg string) string {
maxRemoves := 2
return cleanupPattern.ReplaceAllStringFunc(msg, func(s string) string {
if maxRemoves > 0 {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Logic cleanupPattern replaces those tokens anywhere in the string, so a legitimate reply that mentions “AI:” (e.g. explaining what AI stands for) will have that text silently stripped. Please limit the trim to the leading prefixes (e.g. using line-start anchors or TrimPrefix) so user content is not mangled.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The maxRemoves counter works correctly here since it's declared inside the function and gets reset on each call. However, the intent is unclear—why limit to 2 removals? If a message legitimately contains multiple AI: prefixes, only the first two get removed. Consider adding a comment explaining this limit, or use strings.Replace(msg, pattern, "", 2) for clearer intent.

maxRemoves--
return ""
}
return s
})
}

func formatContextMessageBlock(message string) string {
mrkdwnBlock := slack.NewTextBlockObject("mrkdwn", message, false, false)
contextBlock := slack.NewContextBlock("", []slack.MixedElement{mrkdwnBlock}...)
blockMessage := slack.NewBlockMessage(contextBlock)

jsonByte, err := json.Marshal(blockMessage)
if err != nil {
// Fallback to plain message if marshaling fails
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This silently swallows the error. Consider logging it so you know when marshaling fails.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Added logger.ErrorKV in the marshal error path so failures are surfaced instead of silently dropping the block.

return message
}
return string(jsonByte)
}
9 changes: 6 additions & 3 deletions internal/slack/formatter/formatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,12 @@ func FormatMessage(text string, options FormatOptions) []slack.MsgOption {
case "divider":
slackBlock = slack.NewDividerBlock()
case "context":
var context slack.ContextBlock
if err := json.Unmarshal(blockJSON, &context); err == nil {
slackBlock = context
var contextBlock struct {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix! The previous code tried to unmarshal directly into slack.ContextBlock which doesn't work properly. Extracting Elements and reconstructing with NewContextBlock is the right approach.

Elements slack.ContextElements `json:"elements"`
}

if err := json.Unmarshal(blockJSON, &contextBlock); err == nil {
slackBlock = slack.NewContextBlock("", contextBlock.Elements.Elements...)
}
// Add more block types as needed
}
Expand Down
Loading