Avoid duplicate user prompt and responses in context history

[rangamani54]

• Avoid duplicate of user prompt adding in the context history as user prompt is adding already in the context history when we fetch thread replies
• Avoid adding AI response to the context history when no tool executed, as we add the response without any slack timestamp and it gets duplicated when we fetch thread replies. We don't need to add the response to the context history as anyway it's going to be added to context history from thread replies.

Summary by CodeRabbit

• Bug Fixes
  • Prevented duplicate messages in conversation threads by excluding already-recorded replies and the original post when aggregating thread replies.
  • Stopped recording assistant final responses in cases where no external action was taken, reducing redundant entries in conversation history.

[coderabbitai]

Walkthrough

The Slack client tightens thread-reply deduplication when building conversation history and removes the prior behavior of appending the assistant's finalResponse to history in the LLM no-tool execution path.

Changes

Cohort / File(s)	Summary
Thread-reply deduplication internal/slack/client.go	In handleUserPrompt, when iterating thread replies, skip replies already present in history and exclude the original user message by checking reply.Timestamp != timestamp.
No-tool history mutation removal internal/slack/client.go	In processLLMResponseAndReply, removed the code path that appended the assistant finalResponse to history when no tool was executed, changing history population for the no-tool scenario.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant S as SlackClient
    participant H as HistoryStore
    participant L as LLM

    U->>S: send prompt (timestamp)
    S->>S: fetch thread replies for timestamp
    loop for each reply
        S->>H: check if reply in history
        alt exists in history
            S-->>H: skip reply
        else if reply.Timestamp == timestamp
            S-->>H: skip original message
        else
            S->>H: append reply to history
        end
    end

    S->>L: call LLM with built history
    alt LLM invokes a tool
        L->>S: tool result + assistant response
        S->>H: append tool result and assistant response to history
    else No tool executed
        L->>S: assistant finalResponse
        S-->>H: (no append of assistant finalResponse in this path)
    end
    S->>U: send reply

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review handleUserPrompt thread iteration and deduplication logic for edge cases (timestamps, reply identity).
• Verify processLLMResponseAndReply no-tool path doesn't rely elsewhere on the previously appended finalResponse.

Poem

🐇 I hopped through threads with gentle care,
Skipping echoes and the original there.
When tools stayed silent, I left the trace light,
A tidy history, snug and tight. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "Avoid duplicate user prompt and responses in context history" directly aligns with the changeset's primary objective, which is to prevent duplicate entries in the context history by skipping already-present user prompts and avoiding AI responses without tool execution. The title is concise, clear, and uses specific terminology that accurately describes the main change without vague language or noise. A teammate scanning the repository history would understand from this title that the PR addresses duplicate handling in the context history mechanism.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bfa3de0 and 24c9c53.

📒 Files selected for processing (1)

• 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/client.go

🧠 Learnings (2)

📚 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/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

⏰ 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 (2)

internal/slack/client.go (2)

397-398: Thread reply deduplication logic is correct.

The condition properly prevents duplicate entries by skipping replies already in history and excluding the current message (identified by timestamp) from thread reply processing. The current message will be added separately at Line 417.

Consider clarifying the comment to be more explicit:

-// Check if this reply is already in history and not the original message
+// Check if this reply is already in history and not the current message being processed
 if !existingMessages[reply.Timestamp] && reply.Timestamp != timestamp {

---

698-791: The implementation is functional but uses an inconsistent mixed strategy for history management.

After tracing the lifecycle:

1. Intermediate messages (tool call JSON at line 718, tool result at line 719): Added to history with empty timestamps, used for tracing/context only. Never posted to Slack separately.

2. Final response (line 784): Sent to Slack but NOT added to history in this function. Will be fetched back when the next user message arrives, then added with a real Slack timestamp.

3. Agent path (line 493): Intermediate messages added to history with empty timestamp AND posted to Slack.

Why actual duplicates won't occur: The deduplication logic (line 398) keys entries by SlackTimestamp. Since intermediate messages have SlackTimestamp = "" and Slack replies have real timestamps, they occupy different keys in existingMessages and won't collide.

However, the inconsistency remains: Some assistant responses are in history without timestamps (intermediates), while others have real Slack timestamps (fetched replies). When getContextFromHistory builds context, it includes all entries, potentially creating redundant content if the final response is similar to synthesized context from intermediates.

Recommendation: Clarify the design intent. Either:

• Option 1: Don't persist intermediate tool-related messages to history; keep them for tracing only, and rely entirely on Slack thread fetching for conversation context.
• Option 2: Consistently add all assistant responses immediately with placeholder timestamps, then update them when Slack confirms the real timestamp (requires deduplication by content hash rather than timestamp).

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

internal/slack/client.go (2)

384-384: Consider removing commented-out code.

The commented-out line suggests a previous deduplication approach using a composite key format UserID:Content. Since the current implementation uses SlackTimestamp as the unique identifier (which is more reliable), this commented code should be removed to improve code cleanliness.

Apply this diff to remove the commented code:

-			// key := fmt.Sprintf("%s:%s", msg.UserID, msg.Content)
 			existingMessages[msg.SlackTimestamp] = true

---

388-388: Consider removing commented-out code.

Similar to line 384, this commented-out line should be removed as it references the old deduplication approach.

Apply this diff to remove the commented code:

-			// replyKey := fmt.Sprintf("%s:%s", reply.User, reply.Text)
 			// Check if this reply is already in history and not the original message

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1790fe4 and 874c57c.

📒 Files selected for processing (1)

• 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/client.go

🔇 Additional comments (1)

internal/slack/client.go (1)

389-390: LGTM! The deduplication logic correctly prevents the original message from being added twice.

The additional condition reply.Timestamp != timestamp ensures that when iterating through thread replies, the original message that triggered this handler is excluded. This prevents duplication since the same message is explicitly added to history at line 409.

The logic is sound:

• Thread replies include all messages in the thread (including the current one)
• Without this check, the current message would be added both in this loop and at line 409
• The timestamp comparison effectively filters it out during the loop iteration

[tuannvm]

@cursor code review