feat: Improve agent mode message formatting#150
feat: Improve agent mode message formatting#150ToshihitoKon wants to merge 6 commits intotuannvm:mainfrom
Conversation
|
Warning Rate limit exceeded@ToshihitoKon has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughDetects "thinking" agent outputs at chain end and routes them as Slack context blocks or formats final response text; refactors Slack context block parsing to unmarshal nested elements; injects a logger into the agent callback handler and adds unit tests for the new helpers. Changes
Sequence DiagramsequenceDiagram
autonumber
participant Chain as Chain
participant Handler as agentCallbackHandler
participant Detector as isThinkingMessage
participant Formatter as Formatter
participant SlackAPI as Slack API
Chain->>Handler: onChainEnd(outputs)
Handler->>Detector: isThinkingMessage(outputs["text"])
alt thinking message
Detector-->>Handler: true
Handler->>Formatter: formatContextMessageBlock(text)
Formatter->>SlackAPI: json.Marshal(block)
alt marshal success
SlackAPI-->>Formatter: block JSON
Formatter-->>Handler: context block payload
else marshal failure
SlackAPI-->>Formatter: error
Formatter-->>Handler: fallback plain text (logs error)
end
else final response
Detector-->>Handler: false
Handler->>Formatter: formatFinalResponse(text)
Formatter-->>Handler: final text payload
end
Handler->>SlackAPI: sendMessage(formatted payload)
SlackAPI-->>Handler: send result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Toshihito Kon <toshihito.kon@repro.io>
ToshihitoKon
force-pushed
the
improve-format-agent-mode-context
branch
from
7735d6e to
891bd56
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
internal/slack/agentCallbackHandler.go (3)
32-35: Consider the brittleness of literal pattern matching.The regex patterns are very specific to the current prompt format ("Do I need to use a tool? Yes/No" and "AI:"). If the LLM prompt format changes, these patterns will break silently.
Consider documenting these dependencies or making them configurable.
37-39: Use MatchString to avoid unnecessary allocation.The string-to-bytes conversion is unnecessary and creates an extra allocation.
func isThinkingMessage(msg string) bool { - return thinkingPattern.Match([]byte(msg)) + return thinkingPattern.MatchString(msg) }
41-43: Trim whitespace after pattern removal.After removing the patterns, the result may contain leading/trailing whitespace or extra newlines that should be cleaned up.
+import "strings" + func formatFinalResponse(msg string) string { - return cleanupPattern.ReplaceAllString(msg, "") + return strings.TrimSpace(cleanupPattern.ReplaceAllString(msg, "")) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/slack/agentCallbackHandler.go(1 hunks)internal/slack/formatter/formatter.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
internal/slack/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Slack client implementation and message formatting should be in
internal/slack/
Files:
internal/slack/formatter/formatter.gointernal/slack/agentCallbackHandler.go
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/slack/**/*.go : Slack client implementation and message formatting should be in `internal/slack/`
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/slack/**/*.go : Slack client implementation and message formatting should be in `internal/slack/`
Applied to files:
internal/slack/formatter/formatter.gointernal/slack/agentCallbackHandler.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build Verification
🔇 Additional comments (3)
internal/slack/agentCallbackHandler.go (2)
19-30: LGTM: Clean routing logic for message types.The routing logic clearly separates thinking messages from final responses and applies appropriate formatters to each.
45-56: LGTM: Solid error handling with fallback.The function correctly constructs a Slack block message and provides a sensible fallback if JSON marshaling fails.
internal/slack/formatter/formatter.go (1)
116-122: Code is correct; no issues found.The original concern is unfounded. Verification confirms that
slack.ContextElementshas a nestedElementsfield of type[]MixedElement, andslack.NewContextBlockaccepts variadicMixedElementarguments. The code properly accessescontextBlock.Elements.Elementsand spreads it into the function call—types are fully compatible.
ToshihitoKon
changed the title
- Add detection for thinking messages using regex pattern - Format thinking messages as Slack context blocks for better UX - Clean up final responses by removing AI prefixes - Add JSON marshaling with fallback for block formatting - Change from SimpleHandler to Handler interface 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Toshihito Kon <toshihito.kon@repro.io>
ToshihitoKon
force-pushed
the
improve-format-agent-mode-context
branch
from
891bd56 to
ccbd161
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/slack/agentCallbackHandler.go (2)
32-35: Consider refining the regex patterns.The patterns work but could be improved:
- The capture group in
cleanupPatternis unused—(?:...)would be more efficient if grouping is needed, but the parentheses can be removed entirely here.- The
AI:pattern is very generic and might match unintended occurrences within the response content.Apply this diff if you want to remove the unnecessary capture group:
- cleanupPattern = regexp.MustCompile(`(Do I need to use a tool\? No|AI:)`) + cleanupPattern = regexp.MustCompile(`Do I need to use a tool\? No|AI:`)For more precision, consider anchoring or adding context, e.g.:
cleanupPattern = regexp.MustCompile(`(?m)^(?:Do I need to use a tool\? No|AI:)\s*`)
41-54: Consider cleaning up whitespace after pattern removal.The removal logic correctly limits replacements to 2 (matching the expected format), but it may leave extra whitespace (newlines, spaces) after removing the patterns.
Consider adding a trimming step after replacement:
func formatFinalResponse(msg string) string { maxRemoves := 2 - return cleanupPattern.ReplaceAllStringFunc(msg, func(s string) string { + result := cleanupPattern.ReplaceAllStringFunc(msg, func(s string) string { if maxRemoves > 0 { maxRemoves-- return "" } return s }) + return strings.TrimSpace(result) }Don't forget to import
stringsif you apply this change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/slack/agentCallbackHandler.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
internal/slack/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Slack client implementation and message formatting should be in
internal/slack/
Files:
internal/slack/agentCallbackHandler.go
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/slack/**/*.go : Slack client implementation and message formatting should be in `internal/slack/`
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/slack/**/*.go : Slack client implementation and message formatting should be in `internal/slack/`
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : Implement LangChain integration when adding a new LLM provider
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/handlers/**/*.go : Tool handlers and LLM-MCP bridge should be implemented in `internal/handlers/`
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : LangChain gateway should provide a unified interface for LLM providers
Applied to files:
internal/slack/agentCallbackHandler.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: Security Scan
🔇 Additional comments (4)
internal/slack/agentCallbackHandler.go (4)
5-9: LGTM! Imports support the new formatting features.The added imports (json, regexp, slack) are all necessary for the enhanced message formatting functionality.
19-30: LGTM! Clean message routing logic.The HandleChainEnd method properly handles type assertions and routes messages to the appropriate formatter based on content type.
37-39: LGTM! Clean helper function.Simple and clear utility for detecting thinking messages.
56-67: JSON block format is properly handled end-to-end.The verification confirms the code flow works correctly:
formatContextMessageBlockproduces valid Block Kit JSON with a "blocks" fieldSendMessagedetects this viaformatter.DetectMessageType→isValidBlockKit- Explicit handling exists for
JSONBlocktype at userFrontend.go:190-195- Fallback to plain text is available if Block Kit processing fails (line 219-220)
No action required.
|
Hi @tuannvm ! Could you please take a look at this PR when you have a moment? Your review would be greatly appreciated. Thanks! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| func formatFinalResponse(msg string) string { | ||
| maxRemoves := 2 | ||
| return cleanupPattern.ReplaceAllStringFunc(msg, func(s string) string { | ||
| if maxRemoves > 0 { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
|
||
| jsonByte, err := json.Marshal(blockMessage) | ||
| if err != nil { | ||
| // Fallback to plain message if marshaling fails |
There was a problem hiding this comment.
This silently swallows the error. Consider logging it so you know when marshaling fails.
There was a problem hiding this comment.
✅ Added logger.ErrorKV in the marshal error path so failures are surfaced instead of silently dropping the block.
| @@ -2,7 +2,11 @@ package slackbot | |||
|
|
|||
There was a problem hiding this comment.
Could you add unit tests for the new functions? Specifically:
- isThinkingMessage() with positive/negative cases
- formatFinalResponse() verifying prefix removal
- formatContextMessageBlock() verifying JSON output structure
There was a problem hiding this comment.
✅ Added internal/slack/agentCallbackHandler_test.go covering isThinkingMessage, formatFinalResponse, and verifying the JSON produced by formatContextMessageBlock.
| var context slack.ContextBlock | ||
| if err := json.Unmarshal(blockJSON, &context); err == nil { | ||
| slackBlock = context | ||
| var contextBlock struct { |
There was a problem hiding this comment.
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.
|
@ToshihitoKon some nit picks, please help address, thanks! |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
internal/slack/agentCallbackHandler.go (1)
22-48: Tighten final-response cleanup to avoid mangling content and stray “Thought:” linesRight now
formatFinalResponseblindly replaces the substrings"Do I need to use a tool? No"and"AI:"once, anywhere in the message. That has two side effects:
- A legitimate reply that mentions
AI:(or that question) in the middle of a sentence will be partially stripped.- The “Thought:” line isn’t removed; it becomes
Thought:+ newline + answer, which doesn’t fully meet the “clean up final responses” goal.Consider operating line-by-line, dropping the full “Thought” line and only trimming an
AI:prefix when it appears at the start of a line. For example:-func formatFinalResponse(msg string) string { - msg = strings.Replace(msg, "Do I need to use a tool? No", "", 1) - return strings.Replace(msg, "AI:", "", 1) -} +func formatFinalResponse(msg string) string { + lines := strings.Split(msg, "\n") + cleaned := make([]string, 0, len(lines)) + + for _, line := range lines { + trimmed := strings.TrimSpace(line) + + // Drop the agent thought line entirely + if strings.HasPrefix(trimmed, "Thought:") && + strings.Contains(trimmed, "Do I need to use a tool? No") { + continue + } + + // Remove only a leading "AI:" prefix + trimmed = strings.TrimSpace(strings.TrimPrefix(trimmed, "AI:")) + if trimmed != "" { + cleaned = append(cleaned, trimmed) + } + } + + return strings.TrimSpace(strings.Join(cleaned, "\n")) +}This keeps user content intact while achieving the intended cleanup.
🧹 Nitpick comments (1)
internal/slack/agentCallbackHandler.go (1)
50-62: Guard logger usage in error path for safer fallback
formatContextMessageBlock’s error path unconditionally callslogger.ErrorKV. If a future caller accidentally passes a nil logger, this will panic while trying to handle a non-fatal marshal error.A small guard keeps this helper robust:
func formatContextMessageBlock(message string, logger *logging.Logger) string { @@ - jsonByte, err := json.Marshal(blockMessage) - if err != nil { - // Fallback to plain message if marshaling fails - logger.ErrorKV("Failed to marshal block message", "error", err) - return message - } - return string(jsonByte) + jsonByte, err := json.Marshal(blockMessage) + if err != nil { + // Fallback to plain message if marshaling fails + if logger != nil { + logger.ErrorKV("Failed to marshal block message", "error", err) + } + return message + } + return string(jsonByte) }Given the current call site always passes
c.logger, this is defensive, but it prevents a hard crash if that ever changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/slack/agentCallbackHandler.go(1 hunks)internal/slack/client.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
internal/slack/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Slack client implementation and message formatting should be in
internal/slack/
Files:
internal/slack/agentCallbackHandler.gointernal/slack/client.go
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/slack/**/*.go : Slack client implementation and message formatting should be in `internal/slack/`
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/slack/**/*.go : Slack client implementation and message formatting should be in `internal/slack/`
Applied to files:
internal/slack/agentCallbackHandler.gointernal/slack/client.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : Implement LangChain integration when adding a new LLM provider
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/handlers/**/*.go : Tool handlers and LLM-MCP bridge should be implemented in `internal/handlers/`
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : LangChain gateway should provide a unified interface for LLM providers
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/mcp/**/*.go : Failed servers should be logged but must not crash the application
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/mcp/**/*.go : Component-specific loggers should be used for MCP servers
Applied to files:
internal/slack/agentCallbackHandler.gointernal/slack/client.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : Support both native tools and system prompt-based tools in LLM provider code
Applied to files:
internal/slack/client.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : Update provider constants when adding a new LLM provider
Applied to files:
internal/slack/client.go
🧬 Code graph analysis (1)
internal/slack/agentCallbackHandler.go (1)
internal/common/logging/logger.go (1)
Logger(38-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: code-review
🔇 Additional comments (2)
internal/slack/agentCallbackHandler.go (1)
3-20: Logger injection and Slack-specific imports look appropriateAdding
logging.LoggertoagentCallbackHandlerand importing Slack/JSON/regexp helpers keeps Slack formatting concerns ininternal/slackas intended and provides a clean way to log handler-specific issues. No changes requested.internal/slack/client.go (1)
532-542: Logger wiring intoagentCallbackHandlerlooks correctPassing
c.loggerintoagentCallbackHandlerkeeps logging consistent with the rest of the Slack client and enables the new formatting helper to report marshal failures without changing existing call semantics.
ToshihitoKon
force-pushed
the
improve-format-agent-mode-context
branch
3 times, most recently
from
48968f3 to
e15b83b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
internal/slack/agentCallbackHandler.go (1)
45-49: Consider anchoring prefix removal to prevent mangling legitimate content.The current implementation uses
strings.Replacewith count=1, which removes the first occurrence anywhere in the string. This is better than unlimited replacement but can still remove legitimate content. For example, if the final response is "The AI: prefix is commonly used in chatbots", the colon would be removed, yielding "The prefix is commonly used in chatbots".A previous review comment suggested using line-start anchors or
TrimPrefixto limit removal strictly to leading prefixes. Consider:func formatFinalResponse(msg string) string { - msg = strings.Replace(msg, "Do I need to use a tool? No", "", 1) - msg = strings.Replace(msg, "AI:", "", 1) + msg = strings.TrimPrefix(msg, "Do I need to use a tool? No\n") + msg = strings.TrimPrefix(msg, "AI: ") return strings.TrimSpace(msg) }Or use a regex with
^anchor if multi-line handling is needed. This ensures only actual agent prefixes are removed, not user content that happens to contain those strings.
🧹 Nitpick comments (1)
internal/slack/agentCallbackHandler.go (1)
35-39: Consider a more robust thinking-message detection strategy.The hard-coded regex pattern
Do I need to use a tool\? Yesis tightly coupled to the current LLM agent output format. If the agent prompt template ininternal/llm/langchain.gochanges (e.g., rephrased to "Should I use a tool? Yes" or "Tool usage required: yes"), this detection will silently fail and thinking messages will be sent as final responses.Consider one of the following:
- Extract the pattern as a configurable constant with a comment linking it to the agent template definition.
- Use a more flexible detection heuristic, such as checking for the presence of both "tool" and "Yes" keywords in combination with other agent markers (e.g., "Thought:").
- Add integration tests that verify the agent's actual output format matches this pattern, so format drift is caught early.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/slack/agentCallbackHandler.go(1 hunks)internal/slack/agentCallbackHandler_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/slack/agentCallbackHandler_test.go
🧰 Additional context used
📓 Path-based instructions (1)
internal/slack/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Slack client implementation and message formatting should be in
internal/slack/
Files:
internal/slack/agentCallbackHandler.go
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Agent mode should use multi-turn conversational interactions via LangChain agents and context-aware tool usage decisions
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/slack/**/*.go : Slack client implementation and message formatting should be in `internal/slack/`
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : Implement LangChain integration when adding a new LLM provider
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/handlers/**/*.go : Tool handlers and LLM-MCP bridge should be implemented in `internal/handlers/`
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : LangChain gateway should provide a unified interface for LLM providers
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/mcp/**/*.go : Failed servers should be logged but must not crash the application
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/mcp/**/*_test.go : Integration tests should be written for MCP client functionality
Applied to files:
internal/slack/agentCallbackHandler.go
📚 Learning: 2025-07-20T02:54:03.119Z
Learnt from: CR
Repo: tuannvm/slack-mcp-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/mcp/**/*.go : Component-specific loggers should be used for MCP servers
Applied to files:
internal/slack/agentCallbackHandler.go
🧬 Code graph analysis (1)
internal/slack/agentCallbackHandler.go (1)
internal/common/logging/logger.go (1)
Logger(38-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Verification
- GitHub Check: code-review
🔇 Additional comments (4)
internal/slack/agentCallbackHandler.go (4)
5-12: LGTM!The new imports are all necessary for the added functionality: JSON marshaling for Slack blocks, regex pattern matching for message detection, string manipulation for cleanup, the Slack SDK for block construction, and logging for error handling.
19-19: LGTM!Adding the logger field enables proper error visibility in formatContextMessageBlock and improves testability.
22-33: LGTM!The HandleChainEnd logic properly distinguishes thinking messages from final responses and routes them through appropriate formatting functions. Type checking and flow control are correct.
51-63: LGTM!The function correctly constructs a Slack context block using the slack-go/slack library, marshals it to JSON for transmission, and includes proper error handling with logging and a fallback to plain text. This aligns with the PR's goal of formatting thinking messages as context blocks.
… message Signed-off-by: Kon Toshihito <toshihito.kon@gmail.com>
Signed-off-by: Kon Toshihito <toshihito.kon@gmail.com>
Signed-off-by: Kon Toshihito <toshihito.kon@gmail.com>
ToshihitoKon
force-pushed
the
improve-format-agent-mode-context
branch
from
e15b83b to
2e8b7d8
Compare
2 participants
Summary
Enhance agent mode message formatting to provide better visual distinction between thinking messages and final responses in Slack.
Changes
screenshot
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.