feat: add configurable thought visibility with slash and text commands#95
feat: add configurable thought visibility with slash and text commands#95aadaam wants to merge 8 commits into
Conversation
This commit improves the Slack bot's agent mode interface with better visual formatting and user experience enhancements: **Configuration Changes (internal/config/config.go):** - Updated default thinking message from "Thinking..." to ":thinking_face: _Thinking..._" with emoji and italic formatting **LLM Agent Prompt Improvements (internal/llm/langchain.go):** - Enhanced agent prompt formatting with visual hierarchy using ">" prefixes for thoughts - Clarified tool invocation instructions with better structure and clearer field descriptions - Improved thought/action formatting to be more readable in Slack **Slack Agent Output Formatting (internal/slack/agentCallbackHandler.go):** - Added comprehensive agent output formatting system with two approaches: - Block Kit formatting for rich Slack messages with proper sections, context blocks, and code formatting - Plain text fallback with emojis, visual separators, and structured metadata display - Implemented intelligent parsing of agent responses to extract: - Thoughts with :brain: emoji and italic formatting - Justifications with :scales: emoji - Actions with :right-facing_fist: emoji - Action inputs formatted as code blocks - Observations with :mag: emoji - Added visual separators (───────────) between metadata and AI responses - Proper handling of markdown headers, code blocks, and structured content - Fallback mechanisms to ensure messages always display properly These changes make the agent's reasoning process more transparent and visually appealing in Slack, helping users better understand what the AI is thinking and doing. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Exclude bin/ directory containing build artifacts - Ignore Python files and requirements.txt - Ignore system.prompt file 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add AddReaction and RemoveReaction methods to UserFrontend interface - Implement reaction methods in SlackClient using Slack API - Add stub implementations for StdioClient - Remove code that deleted thinking messages from SendMessage This enables the bot to use emoji reactions for status indicators instead of sending temporary messages. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Rate limit exceeded@aadaam has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 1 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 (7)
WalkthroughThis change introduces per-thread "thinking aloud" mode in Slack, allowing users to toggle agent reasoning visibility via slash or text commands. It adds structured Slack message formatting, supports markdown images, manages thread participation, and updates configuration with a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Slack
participant SlackClient
participant Agent
participant Config
User->>Slack: Sends message (/think_aloud, /think_silent, or normal)
Slack->>SlackClient: Event (message or slash command)
SlackClient->>Config: Get showThoughts setting (per-thread)
alt Slash command
SlackClient->>SlackClient: Update showThoughts state
SlackClient->>Slack: Send confirmation message
else Normal message
SlackClient->>Slack: Add :thinking_face: reaction
SlackClient->>Agent: Send prompt (with context)
Agent->>SlackClient: Return response (with thoughts/metadata)
SlackClient->>SlackClient: Format response (show/hide thoughts)
SlackClient->>Slack: Send formatted message
SlackClient->>Slack: Remove :thinking_face: reaction
SlackClient->>SlackClient: Track thread participation
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
internal/slack/formatter/detector.go (3)
264-264: Consider improving regex robustness for complex URLs.The current regex pattern may not handle URLs with parentheses correctly, which are common in some URLs (e.g., Wikipedia links). Consider using a more robust pattern or additional URL validation.
- imagePattern := regexp.MustCompile(`!\[([^\]]*)\]\((https?://[^\s]+)\)`) + imagePattern := regexp.MustCompile(`!\[([^\]]*)\]\((https?://[^\s)]+(?:\([^\s)]*\))*[^\s)]*)\)`)
275-277: URL cleanup logic could be more robust.The parentheses balancing check is a good approach, but it only handles the simple case where there's one extra closing parenthesis. Consider handling more complex scenarios or documenting the limitations.
You could improve the URL cleanup to handle multiple unbalanced parentheses:
- // If URL ends with a parenthesis that's likely part of the markdown, remove it - if strings.HasSuffix(url, ")") && strings.Count(url, "(") < strings.Count(url, ")") { - url = url[:len(url)-1] - } + // Remove trailing unbalanced closing parentheses + for strings.HasSuffix(url, ")") && strings.Count(url, "(") < strings.Count(url, ")") { + url = url[:len(url)-1] + }
348-351: Consider the fallback condition logic.The condition
len(blocks) == 0 || (len(blocks) == 1 && blocks[0]["type"] == "section")might be overly restrictive. If there's only a text section without images, it might still be valuable to format it as a block rather than falling back to plain text.Consider adjusting the fallback logic:
- // If we have no valid image blocks after validation, return original text - if len(blocks) == 0 || (len(blocks) == 1 && blocks[0]["type"] == "section") { + // If we have no image blocks after validation, return original text + hasImageBlocks := false + for _, block := range blocks { + if block["type"] == "image" { + hasImageBlocks = true + break + } + } + if !hasImageBlocks { return text }internal/config/config.go (1)
46-46: Consider adding default value handling for ShowThoughts.The
ShowThoughtsfield doesn't have default value handling inapplyLLMDefaults(). Consider adding explicit default handling for consistency with other configuration fields.Add default handling in the
applyLLMDefaults()method:func (c *Config) applyLLMDefaults() { if c.LLM.Provider == "" { c.LLM.Provider = ProviderOpenAI } + // Set default for ShowThoughts if not explicitly configured + if c.LLM.ShowThoughts == nil { + showThoughts := true + c.LLM.ShowThoughts = &showThoughts + } + if c.LLM.MaxAgentIterations <= 0 || c.LLM.MaxAgentIterations > 100 {internal/slack/agentCallbackHandler.go (1)
34-226: Consider simplifying the complex formatting logicThe
formatAgentOutputfunction is quite complex with nested conditions and multiple responsibilities. Consider extracting helper functions for better maintainability:+// Helper function to process metadata line +func processMetadataLine(line string, contextElements *[]slack.MixedElement) bool { + trimmedLine := strings.TrimSpace(line) + + if strings.HasPrefix(trimmedLine, "> Thought:") || strings.HasPrefix(trimmedLine, "Thought:") { + thoughtContent := strings.TrimPrefix(trimmedLine, "> ") + thoughtContent = strings.TrimPrefix(thoughtContent, "Thought:") + *contextElements = append(*contextElements, + slack.NewTextBlockObject("mrkdwn", "_Thought:_ "+strings.TrimSpace(thoughtContent), false, false)) + return true + } + // ... similar patterns for other metadata types + return false +} func formatAgentOutput(text string) string { // ... existing setup code ... for _, line := range lines { - // ... existing code block handling ... - - // Process thoughts and metadata as context blocks - if strings.HasPrefix(trimmedLine, "> Thought:") || strings.HasPrefix(trimmedLine, "Thought:") { - addSectionBlock() // Add any pending section - thoughtContent := strings.TrimPrefix(trimmedLine, "> ") - thoughtContent = strings.TrimPrefix(thoughtContent, "Thought:") - contextElements = append(contextElements, - slack.NewTextBlockObject("mrkdwn", "_Thought:_ "+strings.TrimSpace(thoughtContent), false, false)) + if processMetadataLine(line, &contextElements) { + addSectionBlock() // Add any pending section + continue + }internal/slack/client.go (1)
402-419: Consider extracting command patterns to constantsThe command detection logic handles multiple variants well, but the hardcoded strings could be maintained better:
+var ( + silentCommands = []string{ + "think silent", "think quietly", + "/think_silent", "/think_quietly", + "!think_silent", "!think_quietly", + } + aloudCommands = []string{ + "think aloud", "think loud", + "/think_aloud", "/think_loud", + "!think_aloud", "!think_loud", + } +) + +func isCommand(prompt string, commands []string) bool { + lower := strings.ToLower(strings.TrimSpace(prompt)) + for _, cmd := range commands { + if lower == cmd { + return true + } + } + return false +} // Check for thinking mode commands (text or slash style) -if lowerPrompt == "think silent" || lowerPrompt == "think quietly" || - lowerPrompt == "/think_silent" || lowerPrompt == "/think_quietly" || - lowerPrompt == "!think_silent" || lowerPrompt == "!think_quietly" { +if isCommand(trimmedPrompt, silentCommands) { c.setThinkingMode(false, channelID, threadTS) return } -if lowerPrompt == "think aloud" || lowerPrompt == "think loud" || - lowerPrompt == "/think_aloud" || lowerPrompt == "/think_loud" || - lowerPrompt == "!think_aloud" || lowerPrompt == "!think_loud" { +if isCommand(trimmedPrompt, aloudCommands) { c.setThinkingMode(true, channelID, threadTS) return }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.gitignore(1 hunks)internal/config/config.go(2 hunks)internal/llm/langchain.go(1 hunks)internal/slack/agentCallbackHandler.go(2 hunks)internal/slack/client.go(14 hunks)internal/slack/formatter/detector.go(1 hunks)internal/slack/formatter/formatter.go(1 hunks)internal/slack/stdioClient.go(1 hunks)internal/slack/userFrontend.go(3 hunks)schema/config-schema.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
internal/llm/**/*.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
internal/llm/**/*.go: LLM provider factories and LangChain integration should be implemented ininternal/llm/
Factory pattern should be used ininternal/llm/for provider creation
LangChain gateway should provide a unified interface for LLM providers
Environment variables should override config file settings in LLM provider code
Support both native tools and system prompt-based tools in LLM provider code
Adding a new LLM provider requires creating a factory ininternal/llm/
Implement LangChain integration when adding a new LLM provider
Update provider constants when adding a new LLM provider
Files:
internal/llm/langchain.go
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/stdioClient.gointernal/slack/userFrontend.gointernal/slack/client.gointernal/slack/agentCallbackHandler.gointernal/slack/formatter/detector.go
internal/config/**/*.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
Configuration management should be handled in
internal/config/, supporting environment variable overrides
Files:
internal/config/config.go
internal/config/config.go
📄 CodeRabbit Inference Engine (CLAUDE.md)
Add environment variable handling in
config.gowhen adding a new LLM provider
Files:
internal/config/config.go
🧠 Learnings (10)
.gitignore (3)
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to mcp-servers.json : Adding a new MCP server requires updating mcp-servers.json
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to mcp-servers.json : MCP servers must be configured in mcp-servers.json with command/args or URL
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to mcp-servers.json : Configuration files must support both legacy format and new mcpServers format
schema/config-schema.json (1)
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to .env : Environment variables required: SLACK_BOT_TOKEN, SLACK_APP_TOKEN, OPENAI_API_KEY, LLM_PROVIDER
internal/llm/langchain.go (5)
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Standard mode should use single-prompt interactions with tool descriptions in system prompt and direct JSON tool call parsing and execution
internal/slack/formatter/formatter.go (1)
Learnt from: CR
PR: tuannvm/slack-mcp-client#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/
internal/slack/stdioClient.go (2)
Learnt from: CR
PR: tuannvm/slack-mcp-client#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/
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/mcp/**/*.go : Clients should support stdio, HTTP, and SSE transport modes
internal/config/config.go (8)
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/config/config.go : Add environment variable handling in config.go when adding a new LLM provider
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : Environment variables should override config file settings in LLM provider code
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : Adding a new LLM provider requires creating a factory in internal/llm/
Learnt from: CR
PR: tuannvm/slack-mcp-client#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/
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : LLM provider factories and LangChain integration should be implemented in internal/llm/
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
internal/slack/userFrontend.go (1)
Learnt from: CR
PR: tuannvm/slack-mcp-client#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/
internal/slack/client.go (15)
Learnt from: CR
PR: tuannvm/slack-mcp-client#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/
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/mcp/**/*.go : Clients should support stdio, HTTP, and SSE transport modes
Learnt from: AdamShannag
PR: #63
File: internal/mcp/sseClient.go:66-68
Timestamp: 2025-07-08T15:34:53.015Z
Learning: The upstream mcp-go library does not provide typed transport errors and only uses string prefixes like "transport error: " via fmt.Errorf. This requires string matching as a fallback approach for error detection in the SSEMCPClientWithRetry implementation.
Learnt from: AdamShannag
PR: #63
File: internal/mcp/sseClient.go:66-68
Timestamp: 2025-07-08T15:34:53.015Z
Learning: The upstream mcp-go library does not provide typed transport errors and only uses string prefixes like "transport error: " via fmt.Errorf. This requires string matching as a fallback approach for error detection in the SSEMCPClientWithRetry implementation until the library supports typed error wrapping.
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/mcp/**/*.go : Test stdio vs HTTP transport modes when debugging MCP issues
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/mcp/**/*.go : Enable --mcpdebug for MCP client logs when debugging MCP issues
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : LLM provider factories and LangChain integration should be implemented in internal/llm/
Learnt from: CR
PR: tuannvm/slack-mcp-client#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/
Learnt from: CR
PR: tuannvm/slack-mcp-client#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
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : Adding a new LLM provider requires creating a factory in internal/llm/
Learnt from: CR
PR: tuannvm/slack-mcp-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T02:54:03.119Z
Learning: Applies to internal/llm/**/*.go : Factory pattern should be used in internal/llm/ for provider creation
internal/slack/agentCallbackHandler.go (1)
Learnt from: CR
PR: tuannvm/slack-mcp-client#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/
internal/slack/formatter/detector.go (1)
Learnt from: CR
PR: tuannvm/slack-mcp-client#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/
🧬 Code Graph Analysis (3)
internal/llm/langchain.go (1)
internal/handlers/llm_mcp_bridge.go (1)
generateToolPrompt(41-113)
internal/slack/formatter/formatter.go (1)
internal/slack/formatter/formatter_test.go (2)
TestCreateBlockMessage(75-134)TestFormatStructuredData(231-277)
internal/slack/userFrontend.go (2)
internal/slack/formatter/detector.go (2)
HasMarkdownImages(289-292)ConvertMarkdownWithImages(295-366)internal/slack/formatter/formatter.go (4)
DefaultOptions(32-38)BlockFormat(21-21)FormatMessage(60-158)FormatMarkdown(279-292)
🔇 Additional comments (22)
.gitignore (3)
36-38: 👍 Good call on excluding build outputTracking the
bin/directory would only add noise to the history, so this ignore pattern is spot-on.
39-42:*.py+requirements.txtlook overly broad—verify intentIgnoring every Python source file and the lockfile means:
- Any helper scripts, prototypes, or infra tooling written in Python can no longer be committed.
- If
requirements.txtis the canonical dependency manifest for those scripts, it will also be lost to VCS.Unless the repo has a hard rule of “zero Python code checked in,” consider narrowing the patterns to the usual artifacts instead:
-# Python files -*.py -requirements.txt +# Python artifacts +__pycache__/ +*.py[cod]This still keeps byte-code and caches out of the repo while retaining real source and its dependency list.
43-44: Fine to ignore generatedsystem.promptIf this file is produced locally and never edited by hand, excluding it avoids accidental commits.
internal/slack/formatter/detector.go (1)
325-329: Good security practice with URL validation.The URL validation that restricts to
http://andhttps://protocols is a good security measure to prevent potentially malicious protocols.internal/config/config.go (2)
46-46: Good use of pointer type for optional boolean configuration.Using
*boolallows distinguishing between an explicitly setfalsevalue and an unset configuration, which is the correct approach for optional boolean settings.
245-245: Enhanced thinking message improves user experience.The updated thinking message with emoji and markdown formatting (
:thinking_face: _Thinking..._) provides better visual feedback in Slack.internal/slack/formatter/formatter.go (1)
120-125: LGTM! Consistent implementation following established patterns.The image block parsing follows the same pattern as other block types with proper error handling and use of the appropriate
slack.ImageBlocktype from the Slack SDK.internal/llm/langchain.go (1)
212-212: Good formatting enhancement for thought visibility.Adding the
"> "prefix to "Thought:" lines formats them as block quotes in Slack, which improves visual distinction when thoughts are displayed. This aligns well with the configurable thought visibility feature.Also applies to: 222-222
schema/config-schema.json (1)
63-67: Well-structured schema addition with clear documentation.The
showThoughtsproperty is properly defined with:
- Appropriate boolean type
- Sensible default value (
true)- Clear description explaining the feature and its relationship to LLM history
- Consistent placement within the LLM configuration section
internal/slack/stdioClient.go (1)
128-136: LGTM!The
AddReactionandRemoveReactionmethods correctly implement theUserFrontendinterface for the stdio client, following the established pattern of printing actions to standard output.internal/slack/userFrontend.go (3)
27-28: Interface extension looks good!The addition of
AddReactionandRemoveReactionmethods to theUserFrontendinterface properly supports the new reaction-based feedback mechanism.
126-146: Well-implemented reaction methods!Both methods properly handle Slack API calls with appropriate error logging and debug messages. The use of
slack.NewRefToMessageis the correct way to create message references.
179-194: Good enhancement for markdown image support!The logic correctly detects markdown images and converts them to Slack Block Kit format for better rendering. The fallback to regular markdown formatting when no images are present maintains backward compatibility.
internal/slack/agentCallbackHandler.go (4)
16-17: Good design for thought visibility control!The addition of
showThoughtsflag andstoreFullMessagecallback properly supports the configurable visibility feature while preserving full message history.
24-29: Proper implementation of visibility control!The logic correctly stores the full message for history while formatting the display based on the
showThoughtssetting.
228-330: Well-structured plain text formatting!The
formatPlainAgentOutputfunction provides good readability with emojis and visual separators. The handling of code blocks within Action Input is particularly well done.
332-396: Correct implementation of visibility filtering!The
formatPlainAgentOutputWithVisibilityfunction properly filters out thoughts whenshowThoughtsis false, while preserving the actual AI response. The default message "(Response processed)" is a good fallback.internal/slack/client.go (5)
38-39: Good design for thread-aware state management!The use of maps with "channel:threadTS" keys for tracking thread participation and per-thread thought visibility preferences is a clean approach.
308-336: Thread-aware history management looks good!The implementation correctly uses composite keys for thread-specific history while maintaining backward compatibility for non-threaded messages.
427-430: Good UX with thinking emoji reactions!The implementation of adding/removing "thinking_face" reactions provides clear visual feedback during processing. Error handling is appropriate - failures are logged but don't block the main flow.
Also applies to: 450-451, 477-478, 488-489, 590-591
662-680: Well-designed preference cascade!The
getShowThoughtsmethod implements a proper hierarchy: thread preference -> config default -> true. This provides flexibility while maintaining sensible defaults.
267-298: Verify thread participation logic manuallyAutomated inspection didn’t locate unit tests covering
hasParticipatedInThreadortrackThreadParticipation, nor could we extract their implementations via scripts—please confirm the following edge cases are handled:
- Empty or malformed
threadTSvalues (e.g."", non‐numeric, missing decimal)- Race conditions when marking or checking participation concurrently
- Initialization of the participation cache/map before first use
- Expiration or cleanup of old thread entries, if applicable
Additionally, consider adding unit tests in
*_test.gothat:
- Call
trackThreadParticipationthen asserthasParticipatedInThreadreturnstrue- Pass empty or invalid
threadTSvalues to both methods- Simulate concurrent calls to ensure thread‐safe behavior
- Replace SendMessage thinking indicator with AddReaction emoji - Add thinking_face emoji when processing begins - Remove thinking_face emoji when processing completes or errors - Pass message timestamp through to enable reactions on correct message - Update handleUserPrompt and processLLMResponseAndReply signatures This provides a cleaner UX with emoji reactions instead of temporary messages that need to be deleted. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Adam Nemeth <aadaam@gmail.com>
- Add participatedThreads map to track threads where bot has responded - Enable bot to respond to messages in threads it has participated in - Make message history thread-aware using "channel:threadTS" keys - Update addToHistory and getContextFromHistory to handle thread context - Track thread participation when bot sends messages This allows the bot to maintain separate conversation contexts for each thread and respond to follow-up messages without requiring @mentions. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Adam Nemeth <aadaam@gmail.com>
- Detect markdown image syntax (\) in messages - Convert markdown images to Slack Block Kit image blocks - Add support for image blocks in FormatMessage function - Enable rich chart visualization from MCP servers (e.g., QuickChart) This enhancement allows MCP servers that generate chart URLs (like https://github.com/GongRzhe/Quickchart-MCP-Server) to display their visualizations directly in Slack as embedded images instead of just URLs. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Adam Nemeth <aadaam@gmail.com>
- Add showThoughts config option (defaults to true) - Implement /think_aloud and /think_silent slash commands for main channels - Add text command variants for thread support (think silent, \!think_aloud, etc) - Store full thoughts in conversation history while hiding from Slack display - Maintain per-channel/thread preferences for thought visibility This allows users to control whether the LLM's reasoning process is visible while maintaining full context for the LLM to provide better responses. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Adam Nemeth <aadaam@gmail.com>
- Return empty string instead of default message when no AI response content - Only send non-empty messages to Slack in HandleChainEnd - Prevents unnecessary messages appearing during agent thinking 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Adam Nemeth <aadaam@gmail.com>
aadaam
force-pushed
the
feat/thought-visibility
branch
from
f5f52e2 to
f6f89e2
Compare
1 participant
Summary
Changes
showThoughtsconfiguration option inconfig.goand schema (defaults to true)/think_aloudand/think_silentfor main channelsthink silent,\!think_aloud, etc.agentCallbackHandlerto conditionally format output based on visibility settingBenefits
Test Plan
/think_silentslash command in main channel\!think_aloudtext command in thread🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores