Skip to content

fix(platform): make failed status unconditionally terminal and remove dead code #603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Israeltheminer merged 2 commits into from
Feb 28, 2026
Merged

fix(platform): make failed status unconditionally terminal and remove dead code #603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad2c63a
fix(platform): make failed status unconditionally terminal and remove…
Israeltheminer Feb 28, 2026
416e758
Merge branch 'main' into fix/chat-loading-state-failed-status
Israeltheminer Feb 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion services/platform/app/features/chat/components/chat-history-sidebar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ export function ChatHistorySidebar({
}}
className="absolute inset-0 cursor-pointer rounded-md"
/>
<span className="pointer-events-none relative z-10 min-h-[20px] flex-1 truncate text-left text-sm leading-snug">
<span className="pointer-events-none relative z-10 min-h-[1.5rem] flex-1 truncate text-left text-sm leading-snug">
{chat.title}
</span>
<div className="relative z-10 opacity-100 transition-opacity md:opacity-0 md:group-hover:opacity-100">
Expand Down
173 changes: 48 additions & 125 deletions services/platform/app/features/chat/hooks/__tests__/use-chat-loading-state.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ describe('useChatLoadingState', () => {
expect(result.current.isLoading).toBe(false);
});

it('returns true when last part is a tool part (unfinished tool turn)', () => {
it('returns false when failed mid-tool-call (failed is unconditionally terminal)', () => {
const { result } = renderHook(() =>
useChatLoadingState({
isPending: false,
Expand All @@ -132,130 +132,7 @@ describe('useChatLoadingState', () => {
id: 'msg-1',
order: 0,
role: 'assistant',
status: 'success',
text: 'Let me create that for you.',
parts: [
{ type: 'text', text: 'Let me create that for you.' },
{ type: 'step-start' },
{
type: 'tool-excel',
toolCallId: 'call-1',
input: { operation: 'generate' },
state: 'input-available',
},
],
}),
],
threadId: THREAD_A,
pendingThreadId: null,
}),
);

expect(result.current.isLoading).toBe(true);
});

it('returns false when tool parts exist but text part follows (completed tool turn)', () => {
const { result } = renderHook(() =>
useChatLoadingState({
isPending: false,
setIsPending,
uiMessages: [
createUIMessage({
id: 'msg-1',
order: 0,
role: 'assistant',
status: 'success',
text: 'Here are the results...',
parts: [
{ type: 'step-start' },
{
type: 'tool-rag_search',
toolCallId: 'call-1',
input: { query: 'test' },
output: { results: [] },
state: 'output-available',
},
{ type: 'text', text: 'Here are the results...' },
],
}),
],
threadId: THREAD_A,
pendingThreadId: null,
}),
);

expect(result.current.isLoading).toBe(false);
});

it('returns true when tool message has text preamble before tool call', () => {
const { result } = renderHook(() =>
useChatLoadingState({
isPending: false,
setIsPending,
uiMessages: [
createUIMessage({
id: 'msg-1',
order: 0,
role: 'assistant',
status: 'success',
text: 'I will create an Excel file for you.',
parts: [
{
type: 'text',
text: 'I will create an Excel file for you.',
},
{ type: 'step-start' },
{
type: 'tool-excel',
toolCallId: 'call-1',
input: { operation: 'generate' },
state: 'input-available',
},
],
}),
],
threadId: THREAD_A,
pendingThreadId: null,
}),
);

expect(result.current.isLoading).toBe(true);
});

it('returns false when last assistant message is aborted', () => {
const { result } = renderHook(() =>
useChatLoadingState({
isPending: false,
setIsPending,
uiMessages: [
createUIMessage({
id: 'msg-1',
order: 0,
role: 'assistant',
// oxlint-disable-next-line typescript/no-unsafe-type-assertion -- runtime value exists but SDK types lack it
status: 'aborted' as UIMessage['status'],
}),
],
threadId: THREAD_A,
pendingThreadId: null,
}),
);

expect(result.current.isLoading).toBe(false);
});

it('returns false when aborted mid-tool-call (aborted overrides unfinished tool turn)', () => {
const { result } = renderHook(() =>
useChatLoadingState({
isPending: false,
setIsPending,
uiMessages: [
createUIMessage({
id: 'msg-1',
order: 0,
role: 'assistant',
// oxlint-disable-next-line typescript/no-unsafe-type-assertion -- runtime value exists but SDK types lack it
status: 'aborted' as UIMessage['status'],
status: 'failed',
text: 'Let me look that up.',
parts: [
{ type: 'text', text: 'Let me look that up.' },
Expand Down Expand Up @@ -456,6 +333,52 @@ describe('useChatLoadingState', () => {
expect(setIsPending).toHaveBeenCalledWith(false);
});

it('clears isPending when failed mid-tool-call', () => {
const userMsg = createUIMessage({
id: 'msg-1',
order: 0,
role: 'user',
text: 'Hello',
});
const failedToolMsg = createUIMessage({
id: 'msg-2',
order: 1,
role: 'assistant',
text: 'Let me create that for you.',
status: 'failed',
parts: [
{ type: 'text', text: 'Let me create that for you.' },
{ type: 'step-start' },
{
type: 'tool-excel',
toolCallId: 'call-1',
input: { operation: 'generate' },
state: 'input-available',
},
],
});

const { rerender } = renderHook((props) => useChatLoadingState(props), {
initialProps: {
isPending: true,
setIsPending,
uiMessages: [userMsg] as UIMessage[] | undefined,
threadId: THREAD_A as string | undefined,
pendingThreadId: THREAD_A as string | null,
},
});

rerender({
isPending: true,
setIsPending,
uiMessages: [userMsg, failedToolMsg],
threadId: THREAD_A,
pendingThreadId: THREAD_A,
});

expect(setIsPending).toHaveBeenCalledWith(false);
});

it('does not clear isPending while assistant is still streaming', () => {
const userMsg = createUIMessage({
id: 'msg-1',
Expand Down
39 changes: 9 additions & 30 deletions services/platform/app/features/chat/hooks/use-chat-loading-state.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,35 +10,18 @@ interface UseChatLoadingStateParams {
pendingThreadId: string | null;
}

/**
* Checks whether the assistant message represents an in-progress tool turn
* (the final text response has not yet arrived).
*
* The SDK appends parts in message order: tool-call parts first, then
* tool-result merges into existing parts, then final text parts last.
* If the last meaningful part is a tool-* part, the final response
* hasn't arrived yet.
*/
function isUnfinishedToolTurn(message: UIMessage) {
const parts = message.parts;
if (!parts?.length) return false;

for (let i = parts.length - 1; i >= 0; i--) {
const type = parts[i].type;
if (type === 'step-start' || type.startsWith('source-')) continue;
return type.startsWith('tool-');
}
return false;
}

/**
* Derives a single `isLoading` boolean that answers: "Is the AI turn active?"
*
* The AI turn is considered complete (not loading) only when ALL three
* conditions are met:
* The AI turn is considered complete (not loading) only when BOTH conditions
* are met:
* 1. The last message is from the assistant
* 2. The last meaningful part is not a tool part (final text has arrived)
* 3. The last message has a terminal status (success/failed)
* 2. The last message has a terminal status (success/failed)
*
* `failed` is unconditionally terminal — even mid-tool-call — because the
* SDK maps stream abort to `failed` (not `aborted`), and a failed generation
* cannot resume. Without this, a failure during a tool turn would leave
* isLoading stuck forever.
*
* When no messages exist, falls back to `isPending` to bridge the gap
* between send and first subscription data.
Expand All @@ -61,12 +44,8 @@ export function useChatLoadingState({
const status: string | undefined = lastMessage.status;

if (lastMessage.role !== 'assistant') return true;
if (status === 'aborted') return false;

return !(
!isUnfinishedToolTurn(lastMessage) &&
(status === 'success' || status === 'failed')
);
return !(status === 'success' || status === 'failed');
}, [isPending, uiMessages]);

useEffect(() => {
Expand Down
6 changes: 1 addition & 5 deletions services/platform/app/features/chat/hooks/use-message-processing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,7 @@ export function useMessageProcessing(
if (m.status === 'streaming') {
streamingKeysRef.current.add(m.key);
isStreaming = true;
} else if (
m.status === 'success' ||
m.status === 'failed' ||
m.status === 'aborted'
) {
} else if (m.status === 'success' || m.status === 'failed') {
streamingKeysRef.current.delete(m.key);
} else {
isStreaming = streamingKeysRef.current.has(m.key);
Expand Down
5 changes: 0 additions & 5 deletions services/platform/convex/lib/agent_chat/internal_actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,6 @@ export const runAgentGeneration = internalAction({
},
);

// User cancelled — no validation or error handling needed
if (result.finishReason === 'cancelled') {
return result;
}

// Validate response — save a failed message so the client exits loading
if (!result.text?.trim()) {
try {
Expand Down
1 change: 0 additions & 1 deletion services/platform/messages/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2221,7 +2221,6 @@
"loadMore": "Load more",
"loadingMore": "Loading...",
"toast": {
"titleEmpty": "Chat title cannot be empty",
"renameFailed": "Failed to rename chat"
}
},
Expand Down
Loading