♻️ refactor: remove legacy messageLoadingIds from chat store (#13662)

* ♻️ refactor: remove legacy messageLoadingIds from chat store

The messageLoadingIds state and internal_toggleMessageLoading action in the
chat store have been fully superseded by the operation system. The state was
being written to but never read by any consumer — all UI components and
selectors already use operation-based selectors (isMessageGenerating,
isMessageProcessing, etc.).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* 📝 chore: update skill docs to remove messageLoadingIds references

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* 🐛 fix: replace messageLoadingIds with operationSelectors in generation action

The Conversation store's regenerateUserMessage was reading messageLoadingIds
from the chat store to check if a message is already being processed. Replace
with operationSelectors.isMessageProcessing which is the correct way to check
operation state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* 🐛 fix: add operationsByMessage to test mocks for operation selector

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Arvin Xu 2026-04-08 21:54:11 +08:00 committed by GitHub
parent 26d1d6bbfb
commit 8a0c3cb36a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 47 additions and 126 deletions

View file

@ -71,15 +71,18 @@ internal_createTopic: async (params) => {
**Actions:**
- Public: `createTopic`, `sendMessage`
- Internal: `internal_createTopic`, `internal_updateMessageContent`
- Dispatch: `internal_dispatchTopic`
- Toggle: `internal_toggleMessageLoading`
**State:**
**State:**
- ID arrays: `topicEditingIds`
- ID arrays: `messageLoadingIds`, `topicEditingIds`
- Maps: `topicMaps`, `messagesMap`
- Active: `activeTopicId`
- Init flags: `topicsInit`
## Detailed Guides

View file

@ -30,16 +30,13 @@ internal_createMessage: async (message, context) => {
let tempId = context?.tempMessageId;
if (!tempId) {
tempId = internal_createTmpMessage(message);
internal_toggleMessageLoading(true, tempId);
}
try {
const id = await messageService.createMessage(message);
await refreshMessages();
internal_toggleMessageLoading(false, tempId);
return id;
} catch (e) {
internal_toggleMessageLoading(false, tempId);
internal_dispatchMessage({
id: tempId,
type: 'updateMessage',

View file

@ -31,7 +31,8 @@ vi.mock('@/store/chat', () => ({
],
},
operations: {},
messageLoadingIds: [],
operationsByMessage: {},
cancelOperations: mockCancelOperations,
cancelOperation: mockCancelOperation,
cancelSendMessageInServer: mockCancelSendMessageInServer,
@ -163,6 +164,7 @@ describe('Generation Actions', () => {
vi.mocked(await import('@/store/chat').then((m) => m.useChatStore.getState)).mockReturnValue({
messagesMap: {},
operations: {},
operationsByMessage: {},
startOperation: mockStartOperation,
completeOperation: mockCompleteOperation,
failOperation: mockFailOperation,
@ -221,6 +223,7 @@ describe('Generation Actions', () => {
vi.mocked(await import('@/store/chat').then((m) => m.useChatStore.getState)).mockReturnValue({
messagesMap: {},
operations: {},
operationsByMessage: {},
startOperation: mockStartOperation,
completeOperation: mockCompleteOperation,
failOperation: mockFailOperation,
@ -256,6 +259,7 @@ describe('Generation Actions', () => {
vi.mocked(await import('@/store/chat').then((m) => m.useChatStore.getState)).mockReturnValue({
messagesMap: {},
operations: {},
operationsByMessage: {},
startOperation: mockStartOperation,
completeOperation: mockCompleteOperation,
failOperation: mockFailOperation,
@ -293,6 +297,7 @@ describe('Generation Actions', () => {
vi.mocked(await import('@/store/chat').then((m) => m.useChatStore.getState)).mockReturnValue({
messagesMap: {},
operations: {},
operationsByMessage: {},
startOperation: mockStartOperation,
completeOperation: mockCompleteOperation,
failOperation: mockFailOperation,
@ -338,6 +343,7 @@ describe('Generation Actions', () => {
vi.mocked(await import('@/store/chat').then((m) => m.useChatStore.getState)).mockReturnValue({
messagesMap: {},
operations: {},
operationsByMessage: {},
startOperation: mockStartOperation,
completeOperation: mockCompleteOperation,
failOperation: mockFailOperation,
@ -381,6 +387,7 @@ describe('Generation Actions', () => {
vi.mocked(await import('@/store/chat').then((m) => m.useChatStore.getState)).mockReturnValue({
messagesMap: {},
operations: {},
operationsByMessage: {},
startOperation: mockStartOperation,
completeOperation: mockCompleteOperation,
failOperation: mockFailOperation,
@ -419,7 +426,8 @@ describe('Generation Actions', () => {
vi.mocked(useChatStore.getState).mockReturnValue({
messagesMap: {},
operations: {},
messageLoadingIds: [],
operationsByMessage: {},
cancelOperations: mockCancelOperations,
cancelOperation: mockCancelOperation,
deleteMessage: mockDeleteMessage,
@ -482,7 +490,8 @@ describe('Generation Actions', () => {
vi.mocked(useChatStore.getState).mockReturnValue({
messagesMap: {},
operations: {},
messageLoadingIds: [],
operationsByMessage: {},
cancelOperations: mockCancelOperations,
cancelOperation: mockCancelOperation,
deleteMessage: vi.fn().mockImplementation(() => {
@ -550,7 +559,8 @@ describe('Generation Actions', () => {
vi.mocked(useChatStore.getState).mockReturnValue({
messagesMap: {},
operations: {},
messageLoadingIds: [],
operationsByMessage: {},
startOperation: mockStartOperation,
completeOperation: mockCompleteOperation,
deleteMessage: mockDeleteMessage,
@ -590,7 +600,8 @@ describe('Generation Actions', () => {
vi.mocked(useChatStore.getState).mockReturnValue({
messagesMap: {},
operations: {},
messageLoadingIds: [],
operationsByMessage: {},
cancelOperations: mockCancelOperations,
cancelOperation: mockCancelOperation,
deleteMessage: mockDeleteMessage,
@ -641,7 +652,8 @@ describe('Generation Actions', () => {
vi.mocked(useChatStore.getState).mockReturnValue({
messagesMap: {},
operations: {},
messageLoadingIds: [],
operationsByMessage: {},
cancelOperations: mockCancelOperations,
cancelOperation: mockCancelOperation,
deleteMessage: mockDeleteMessage,
@ -697,7 +709,8 @@ describe('Generation Actions', () => {
vi.mocked(useChatStore.getState).mockReturnValue({
messagesMap: {},
operations: {},
messageLoadingIds: [],
operationsByMessage: {},
cancelOperations: mockCancelOperations,
cancelOperation: mockCancelOperation,
deleteMessage: mockDeleteMessage,
@ -744,7 +757,8 @@ describe('Generation Actions', () => {
vi.mocked(useChatStore.getState).mockReturnValue({
messagesMap: {},
operations: {},
messageLoadingIds: [],
operationsByMessage: {},
cancelOperations: mockCancelOperations,
cancelOperation: mockCancelOperation,
deleteMessage: mockDeleteMessage,
@ -811,11 +825,19 @@ describe('Generation Actions', () => {
});
it('should not regenerate if message is already loading', async () => {
// Mock messageLoadingIds to include the target message
// Mock operation system to indicate message is processing
const { useChatStore } = await import('@/store/chat');
vi.mocked(useChatStore.getState).mockReturnValue({
messagesMap: {},
messageLoadingIds: ['msg-1'],
operations: {
'op-1': {
id: 'op-1',
type: 'sendMessage',
status: 'running',
context: { messageIds: ['msg-1'] },
},
},
operationsByMessage: { 'msg-1': ['op-1'] },
startOperation: mockStartOperation,
} as any);

View file

@ -8,6 +8,7 @@ import {
parseSelectedSkillsFromEditorData,
parseSelectedToolsFromEditorData,
} from '@/store/chat/slices/aiChat/actions/commandBus';
import { operationSelectors } from '@/store/chat/slices/operation/selectors';
import { INPUT_LOADING_OPERATION_TYPES } from '@/store/chat/slices/operation/types';
import { type Store as ConversationStore } from '../../action';
@ -314,8 +315,8 @@ export const generationSlice: StateCreator<
const { context, displayMessages, hooks } = get();
const chatStore = useChatStore.getState();
// Check if already regenerating
const isRegenerating = chatStore.messageLoadingIds.includes(messageId);
// Check if already regenerating via operation system
const isRegenerating = operationSelectors.isMessageProcessing(messageId)(chatStore);
if (isRegenerating) return;
// Find the message in current conversation messages

View file

@ -112,7 +112,6 @@ describe('agentGroup actions', () => {
act(() => {
useChatStore.setState({
optimisticCreateTmpMessage: vi.fn(),
internal_toggleMessageLoading: vi.fn(),
internal_dispatchMessage: vi.fn(),
internal_handleAgentStreamEvent: vi.fn(),
internal_cleanupAgentOperation: vi.fn(),
@ -667,12 +666,6 @@ describe('agentGroup actions', () => {
message: TEST_CONTENT.GROUP_MESSAGE,
});
});
// Should toggle loading off in finally block
expect(result.current.internal_toggleMessageLoading).toHaveBeenLastCalledWith(
false,
expect.any(String),
);
});
it('should handle abort error without calling failOperation', async () => {

View file

@ -62,7 +62,6 @@ describe('runAgent actions', () => {
act(() => {
useChatStore.setState({
internal_dispatchMessage: vi.fn(),
internal_toggleMessageLoading: vi.fn(),
optimisticUpdateMessageContent: vi.fn(),
refreshMessages: vi.fn(),
updateOperationMetadata: vi.fn(),

View file

@ -89,10 +89,6 @@ export class ChatGroupChatActionImpl {
{ operationId: execOperationId, tempMessageId: tempAssistantId },
);
// Start loading state for temp messages
this.#get().internal_toggleMessageLoading(true, tempUserId);
this.#get().internal_toggleMessageLoading(true, tempAssistantId);
try {
// 2. Call backend execGroupAgent - creates messages and triggers Agent
// Pass AbortSignal to allow cancellation during the API call
@ -153,8 +149,6 @@ export class ChatGroupChatActionImpl {
message: result.error || 'Agent operation failed to start',
type: 'AgentStartupError',
});
// Stop loading state for assistant message
this.#get().internal_toggleMessageLoading(false, result.assistantMessageId);
return;
}
@ -254,8 +248,6 @@ export class ChatGroupChatActionImpl {
});
}
} finally {
this.#get().internal_toggleMessageLoading(false, tempUserId);
this.#get().internal_toggleMessageLoading(false, tempAssistantId);
this.#set({ isCreatingMessage: false }, false, n('sendGroupMessage/end'));
}
};

View file

@ -73,8 +73,6 @@ export class AgentActionImpl {
});
// Stop loading state
this.#get().internal_toggleMessageLoading(false, assistantId);
// Clean up operation (this will cancel the operation)
this.#get().internal_cleanupAgentOperation(assistantId);
};
@ -131,7 +129,7 @@ export class AgentActionImpl {
// Stop loading state
log(`Stopping loading for completed agent runtime: ${assistantId}`);
this.#get().internal_toggleMessageLoading(false, assistantId);
break;
}
@ -235,7 +233,6 @@ export class AgentActionImpl {
// Stop loading state
log(`Stopping loading for ${assistantId}`);
this.#get().internal_toggleMessageLoading(false, assistantId);
// Show desktop notification
if (isDesktop) {
@ -290,7 +287,6 @@ export class AgentActionImpl {
// Stop loading state, waiting for human intervention
log(`Stopping loading for human approval: ${assistantId}`);
this.#get().internal_toggleMessageLoading(false, assistantId);
} else if (phase === 'tool_execution' && toolCall) {
log(`Tool execution started for ${assistantId}: ${toolCall.function?.name}`);
}
@ -312,7 +308,6 @@ export class AgentActionImpl {
});
log(`Stopping loading for completed agent: ${assistantId}`);
this.#get().internal_toggleMessageLoading(false, assistantId);
}
break;
}
@ -362,9 +357,6 @@ export class AgentActionImpl {
operationId: messageOpId,
});
// Resume loading state
this.#get().internal_toggleMessageLoading(true, assistantId);
// Clear human intervention state
this.#get().updateOperationMetadata(messageOpId, {
needsHumanInput: false,

View file

@ -329,7 +329,6 @@ export class ConversationLifecycleActionImpl {
},
{ operationId, tempMessageId: tempAssistantId },
);
this.#get().internal_toggleMessageLoading(true, tempId);
// Associate temp messages with operation
this.#get().associateMessageWithOperation(tempId, operationId);
@ -495,8 +494,6 @@ export class ConversationLifecycleActionImpl {
}
}
this.#get().internal_toggleMessageLoading(false, tempId);
// Clear editor temp state after message created
if (data) {
this.#get().updateOperationMetadata(operationId, { inputEditorTempState: null });

View file

@ -781,31 +781,6 @@ describe('chatMessage actions', () => {
});
});
describe('internal_toggleMessageLoading', () => {
it('should add message id to messageLoadingIds when loading is true', () => {
const { result } = renderHook(() => useChatStore());
const messageId = 'message-id';
act(() => {
result.current.internal_toggleMessageLoading(true, messageId);
});
expect(result.current.messageLoadingIds).toContain(messageId);
});
it('should remove message id from messageLoadingIds when loading is false', () => {
const { result } = renderHook(() => useChatStore());
const messageId = 'ddd-id';
act(() => {
result.current.internal_toggleMessageLoading(true, messageId);
result.current.internal_toggleMessageLoading(false, messageId);
});
expect(result.current.messageLoadingIds).not.toContain(messageId);
});
});
describe('modifyMessageContent', () => {
it('should call internal_traceMessage with correct parameters before updating', async () => {
const messageId = 'message-id';

View file

@ -55,17 +55,11 @@ export class MessageOptimisticUpdateActionImpl {
tempMessageId?: string;
},
): Promise<{ id: string; messages: UIChatMessage[] } | undefined> => {
const {
optimisticCreateTmpMessage,
internal_toggleMessageLoading,
internal_dispatchMessage,
replaceMessages,
} = this.#get();
const { optimisticCreateTmpMessage, internal_dispatchMessage, replaceMessages } = this.#get();
let tempId = context?.tempMessageId;
if (!tempId) {
tempId = optimisticCreateTmpMessage(message as any, context);
internal_toggleMessageLoading(true, tempId);
}
try {
@ -75,10 +69,8 @@ export class MessageOptimisticUpdateActionImpl {
const ctx = this.#get().internal_getConversationContext(context);
replaceMessages(result.messages, { context: ctx });
internal_toggleMessageLoading(false, tempId);
return result;
} catch (e) {
internal_toggleMessageLoading(false, tempId);
internal_dispatchMessage(
{
id: tempId,

View file

@ -61,16 +61,6 @@ export class MessageRuntimeStateActionImpl {
window.removeEventListener('beforeunload', preventLeavingFn);
}
};
internal_toggleMessageLoading = (loading: boolean, id: string): void => {
this.#set(
{
messageLoadingIds: toggleBooleanList(this.#get().messageLoadingIds, id, loading),
},
false,
`internal_toggleMessageLoading/${loading ? 'start' : 'end'}`,
);
};
}
export type MessageRuntimeStateAction = Pick<

View file

@ -17,10 +17,6 @@ export interface ChatMessageState {
* is the message is editing
*/
messageEditingIds: string[];
/**
* is the message is creating or updating in the service
*/
messageLoadingIds: string[];
/**
* whether messages have fetched
*/
@ -37,7 +33,6 @@ export const initialMessageState: ChatMessageState = {
groupAgentMaps: {},
isCreatingMessage: false,
messageEditingIds: [],
messageLoadingIds: [],
messagesInit: false,
messagesMap: {},
};

View file

@ -1,23 +1,15 @@
import { type ChatStoreState } from '../../../initialState';
import { operationSelectors } from '../../operation/selectors';
import { mainDisplayChatIDs } from './chat';
import { getDbMessageByToolCallId } from './dbMessage';
import { getDisplayMessageById } from './displayMessage';
const isMessageEditing = (id: string) => (s: ChatStoreState) => s.messageEditingIds.includes(id);
/**
* Check if a message is in loading state
* Priority: operation system (for AI flows) > legacy messageLoadingIds (for CRUD operations)
* Check if a message is in loading state via the operation system
*/
const isMessageLoading = (id: string) => (s: ChatStoreState) => {
// First check operation system (sendMessage, etc.)
const hasOperation = operationSelectors.isMessageProcessing(id)(s);
if (hasOperation) return true;
// Fallback to legacy loading state (for non-operation CRUD)
return s.messageLoadingIds.includes(id);
};
const isMessageLoading = (id: string) => (s: ChatStoreState) =>
operationSelectors.isMessageProcessing(id)(s);
// Use operation system for AI-related loading states
const isMessageRegenerating = (id: string) => (s: ChatStoreState) =>
@ -70,23 +62,8 @@ const isToolApiNameShining =
const isCreatingMessage = (s: ChatStoreState) => s.isCreatingMessage;
const isHasMessageLoading = (s: ChatStoreState) =>
s.messageLoadingIds.some((id) => mainDisplayChatIDs(s).includes(id));
/**
* this function is used to determine whether the send button should be disabled
*/
const isSendButtonDisabledByMessage = (s: ChatStoreState) =>
// 1. when there is message loading
isHasMessageLoading(s) ||
// 2. when is creating the topic
s.creatingTopic ||
// 3. when is creating the message
isCreatingMessage(s);
export const messageStateSelectors = {
isCreatingMessage,
isHasMessageLoading,
isInToolsCalling,
isMessageCollapsed,
isMessageContinuing,
@ -97,7 +74,6 @@ export const messageStateSelectors = {
isMessageLoading,
isMessageRegenerating,
isPluginApiInvoking,
isSendButtonDisabledByMessage,
isToolApiNameShining,
isToolCallStreaming,
};

View file

@ -210,14 +210,11 @@ export class PluginOptimisticUpdateActionImpl {
const message = dbMessageSelectors.getDbMessageById(id)(this.#get());
if (!message || !message.tools) return;
const { internal_toggleMessageLoading, replaceMessages, internal_getConversationContext } =
this.#get();
const { replaceMessages, internal_getConversationContext } = this.#get();
const ctx = internal_getConversationContext(context);
internal_toggleMessageLoading(true, id);
const result = await messageService.updateMessage(id, { tools: message.tools }, ctx);
internal_toggleMessageLoading(false, id);
if (result?.success && result.messages) {
replaceMessages(result.messages, { context: ctx });