Skip to content

Commit e1b9be1

Browse files
daniel-lxsdaniel-lxs
committed
fix: resolve chat message edit/delete duplication issues
- Implement timestamp-based fallback for API history truncation - Fix message duplication when editing chat messages - Fix 'Couldn't find timestamp' error when deleting messages - Add UI state refresh after message deletion - Add comprehensive test coverage for delete functionality - Add detailed logging for debugging message operations Fixes issues where edited messages appeared twice in exported history and deletion operations failed with timestamp errors.
1 parent 2571781 commit e1b9be1

File tree

5 files changed

+955
-21
lines changed

5 files changed

+955
-21
lines changed

‎CHAT_MESSAGE_FIX_SUMMARY.md‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
# Chat Message Edit/Delete Fix Summary
2+
3+
**Date**: December 8, 2024
4+
**Branch**: `fix/chat-message-edit-delete-duplication`
5+
6+
## Problem Statement
7+
8+
Users reported two critical issues with chat message operations:
9+
10+
1. **Message Editing Bug**: When editing a chat message, the exported chat history contained duplicated entries. The edited message appeared twice instead of replacing the original message.
11+
12+
2. **Message Deletion Bug**: When attempting to delete a message, users encountered:
13+
- "Couldn't find timestamp" error messages
14+
- Messages not being deleted at all
15+
- UI not refreshing after successful deletion
16+
17+
## Root Cause Analysis
18+
19+
### Edit Issue
20+
21+
The core problem was in `handleEditMessageConfirm` in `webviewMessageHandler.ts`. When `apiConversationHistoryIndex` was -1 (message not found in API history), the code wasn't properly truncating the API conversation history, leading to duplicated entries.
22+
23+
### Delete Issue
24+
25+
Similar to the edit issue, `handleDeleteMessageConfirm` suffered from the same problem when `apiConversationHistoryIndex` was -1. Additionally, the UI wasn't being notified after successful deletion.
26+
27+
## Solution Implemented
28+
29+
### 1. Timestamp-Based Fallback for API History Truncation
30+
31+
Added fallback logic to both edit and delete operations when the exact API history index cannot be found:
32+
33+
```typescript
34+
if (apiConversationHistoryIndex === -1 && currentCline.apiConversationHistory.length > 0) {
35+
// Find the first API message with timestamp >= messageTs
36+
const fallbackIndex = currentCline.apiConversationHistory.findIndex((msg: ApiMessage) => {
37+
return msg.ts && msg.ts >= messageTs
38+
})
39+
40+
if (fallbackIndex !== -1) {
41+
effectiveApiIndex = fallbackIndex
42+
console.log(`Using timestamp-based fallback for API history truncation. Index: ${effectiveApiIndex}`)
43+
}
44+
}
45+
```
46+
47+
This ensures that when editing or deleting a user message, any subsequent assistant responses are also removed from the API history.
48+
49+
### 2. UI Update After Deletion
50+
51+
Added explicit UI state refresh after message deletion:
52+
53+
```typescript
54+
// Update the UI to reflect the deletion
55+
await provider.postStateToWebview()
56+
```
57+
58+
This was necessary because, unlike edit operations (which naturally trigger UI updates by processing new messages), deletions don't have a built-in UI update mechanism.
59+
60+
### 3. Comprehensive Logging
61+
62+
Added detailed logging throughout the delete message flow to aid in debugging:
63+
64+
- Message lookup and index finding
65+
- API history truncation decisions
66+
- UI update triggers
67+
68+
## Files Modified
69+
70+
### Core Implementation Files
71+
72+
1. **`src/core/webview/webviewMessageHandler.ts`**
73+
74+
- Added timestamp-based fallback in `handleEditMessageConfirm` (lines 383-396)
75+
- Added timestamp-based fallback in `removeMessagesThisAndSubsequent` (lines 104-121)
76+
- Added UI update after deletion (line 267)
77+
- Added comprehensive logging for debugging
78+
79+
2. **`src/core/webview/ClineProvider.ts`**
80+
- Applied same timestamp-based fallback logic for checkpoint-driven edits (lines 916-933)
81+
82+
### Test Files
83+
84+
3. **`src/test/webviewMessageHandler.delete.spec.ts`** (NEW)
85+
- Created comprehensive test suite with 5 test cases
86+
- Tests cover normal deletion, fallback scenarios, and edge cases
87+
- All tests passing
88+
89+
## Test Coverage
90+
91+
Created test suite covering:
92+
93+
- ✅ Normal message deletion with valid indices
94+
- ✅ Timestamp-based fallback when API index is -1
95+
- ✅ Handling of messages not found in API history
96+
- ✅ Complete message history clearing
97+
- ✅ UI state updates after deletion
98+
99+
## Verification
100+
101+
All tests pass successfully:
102+
103+
```
104+
✓ webviewMessageHandler - Delete Message (5 tests)
105+
✓ should delete message and all subsequent messages
106+
✓ should use timestamp-based fallback when apiConversationHistoryIndex is -1
107+
✓ should handle delete when message not in API history
108+
✓ should handle delete when no messages exist after target
109+
✓ should update UI state after deletion
110+
```
111+
112+
## Impact
113+
114+
This fix ensures:
115+
116+
- Chat message edits now correctly replace the original message without duplication
117+
- Message deletion works reliably without errors
118+
- The UI properly reflects changes after deletion
119+
- Better error handling and debugging capabilities through comprehensive logging
120+
121+
## Future Considerations
122+
123+
1. Consider adding a confirmation dialog for message deletion in the UI
124+
2. Add telemetry to track successful edit/delete operations
125+
3. Consider batch operations for multiple message deletions
126+
4. Add undo/redo functionality for message operations

‎src/core/webview/ClineProvider.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -917,9 +917,26 @@ export class ClineProvider
917917
// Remove the target message and all subsequent messages
918918
await task.overwriteClineMessages(task.clineMessages.slice(0, messageIndex))
919919

920-
if (apiConversationHistoryIndex !== -1) {
920+
// Apply timestamp-based fallback if exact API index not found
921+
let effectiveApiIndex = apiConversationHistoryIndex
922+
if (apiConversationHistoryIndex === -1 && task.apiConversationHistory.length > 0) {
923+
// Find the first API message with timestamp >= messageTs
924+
// This ensures we remove any assistant responses that came after the edited user message
925+
const fallbackIndex = task.apiConversationHistory.findIndex((msg: any) => {
926+
return msg.ts && msg.ts >= pendingEdit.messageTs
927+
})
928+
929+
if (fallbackIndex !== -1) {
930+
effectiveApiIndex = fallbackIndex
931+
this.log(
932+
`[createTaskWithHistoryItem] Using timestamp-based fallback for API history truncation. Index: ${effectiveApiIndex}`,
933+
)
934+
}
935+
}
936+
937+
if (effectiveApiIndex !== -1) {
921938
await task.overwriteApiConversationHistory(
922-
task.apiConversationHistory.slice(0, apiConversationHistoryIndex),
939+
task.apiConversationHistory.slice(0, effectiveApiIndex),
923940
)
924941
}
925942

0 commit comments

Comments
 (0)