feat(ai-chat): close the error and abort edge-case gaps - #791
Conversation
Old transcripts can carry data parts from retired features. Counting them as displayable made such messages render as a blank bubble instead of falling through to the empty-response affordance. Claude-Session: https://claude.ai/code/session_01S9YxTH6xRfC6bq3vbgmTQD
Stopping a response while it was still pending left the user message as the transcript tail with no indicator and no way to retry short of retyping. Surface an inline notice with the existing Try again action; mid-stream stops keep their partial reply and stay quiet. Claude-Session: https://claude.ai/code/session_01S9YxTH6xRfC6bq3vbgmTQD
Creation failures only hit the console, so the New chat button appeared to do nothing. Match the delete-thread failure toast. Claude-Session: https://claude.ai/code/session_01S9YxTH6xRfC6bq3vbgmTQD
The overflow banner told users to start a new chat but offered no way to do it. Add the action next to Try again; it creates the thread and carries the failed prompt over as a queued direct prompt (text only). Claude-Session: https://claude.ai/code/session_01S9YxTH6xRfC6bq3vbgmTQD
Unclassified failures rendered only generic copy while the real reason (usage-limit reset date, recovery outcome) sat in a thread-list tooltip; surface it as a detail line under the banner headline. The terminal connection banner also told users to refresh with no way to do it — give it a button. Claude-Session: https://claude.ai/code/session_01S9YxTH6xRfC6bq3vbgmTQD
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 5 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR adds aborted-run error states, preserves server error details, and updates AI chat recovery actions. It wires context-overflow new-chat handling through the chat panel, excludes ChangesAI chat error recovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change improves user-facing handling for stopped, failed, and disconnected chat runs without any actionable merge-blocking risk remaining. Sequence Diagram(s)sequenceDiagram
participant AiChatThreadView
participant deriveAiChatAssistantErrorState
participant AiChatMessageList
participant AiChatAssistantError
participant useAiChatPanelController
AiChatThreadView->>deriveAiChatAssistantErrorState: Pass latest message role
deriveAiChatAssistantErrorState-->>AiChatThreadView: Return assistant error state
AiChatThreadView->>AiChatMessageList: Pass error state and onStartNewChat
AiChatMessageList->>AiChatAssistantError: Render error actions
AiChatAssistantError->>useAiChatPanelController: Invoke onStartNewChat
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 8 files
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/workspaces/components/ai-chat/useAiChatPanelController.ts">
<violation number="1" location="src/features/workspaces/components/ai-chat/useAiChatPanelController.ts:56">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
The `handleNewChat` changes add two new behaviors — a `carryPrompt` path that queues a direct prompt after thread creation, and a toast error path on creation failure. Neither is exercised by tests, and there are no tests for `useAiChatPanelController` at all. Add regression assertions covering these paths so the behavior-change is validated.</violation>
</file>
<file name="src/features/workspaces/components/ai-chat/AiChatMessageList.tsx">
<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatMessageList.tsx:307">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
This behavior-change diff introduces several new error-rendering branches in the component (aborted notice, error-detail line, Refresh page button, Start new chat button), but none are exercised by tests. The existing `ai-chat-error-state.test.ts` validates the derivation of the error state objects, not the UI rendering in this file. Add component-level tests that assert the correct buttons and messages render for each `errorState.kind` and `classification` combination.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| try { | ||
| const thread = await createThread(); | ||
| if (carryPrompt) { |
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
The handleNewChat changes add two new behaviors — a carryPrompt path that queues a direct prompt after thread creation, and a toast error path on creation failure. Neither is exercised by tests, and there are no tests for useAiChatPanelController at all. Add regression assertions covering these paths so the behavior-change is validated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/useAiChatPanelController.ts, line 56:
<comment>The `handleNewChat` changes add two new behaviors — a `carryPrompt` path that queues a direct prompt after thread creation, and a toast error path on creation failure. Neither is exercised by tests, and there are no tests for `useAiChatPanelController` at all. Add regression assertions covering these paths so the behavior-change is validated.</comment>
<file context>
@@ -45,16 +46,19 @@ export function useAiChatPanelController({ workspaceId }: UseAiChatPanelControll
try {
const thread = await createThread();
+ if (carryPrompt) {
+ useWorkspaceAiComposerDraftStore.getState().queueDirectPrompt(thread.id, carryPrompt);
+ }
</file context>
| }) { | ||
| // An overflowed chat can rarely be retried into success, so the escape | ||
| // hatch the copy suggests gets its own action. | ||
| const canStartNewChat = |
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
This behavior-change diff introduces several new error-rendering branches in the component (aborted notice, error-detail line, Refresh page button, Start new chat button), but none are exercised by tests. The existing ai-chat-error-state.test.ts validates the derivation of the error state objects, not the UI rendering in this file. Add component-level tests that assert the correct buttons and messages render for each errorState.kind and classification combination.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatMessageList.tsx, line 307:
<comment>This behavior-change diff introduces several new error-rendering branches in the component (aborted notice, error-detail line, Refresh page button, Start new chat button), but none are exercised by tests. The existing `ai-chat-error-state.test.ts` validates the derivation of the error state objects, not the UI rendering in this file. Add component-level tests that assert the correct buttons and messages render for each `errorState.kind` and `classification` combination.</comment>
<file context>
@@ -284,12 +294,25 @@ function AiChatAssistantError({
}) {
+ // An overflowed chat can rarely be retried into success, so the escape
+ // hatch the copy suggests gets its own action.
+ const canStartNewChat =
+ errorState.kind === "assistant" &&
+ errorState.classification === "context_overflow" &&
</file context>
The overflow action now just creates and selects a fresh thread; the queued-direct-prompt plumbing wasn't worth its wiring. Claude-Session: https://claude.ai/code/session_01S9YxTH6xRfC6bq3vbgmTQD
9931e73 to
2562d16
Compare
Switching threads suspended on the /get-messages fetch every time, even for a thread visited seconds ago. Serve the last settled transcript from an in-session cache via a pre-fulfilled thenable (React 19 unwraps it synchronously, so no fallback frame); the server's connect-time broadcast replaces it with authoritative state one round trip later. Streaming threads are never cached — the resume handshake is their fast path, and a seeded partial risks a duplicate bubble on message-id drift. Claude-Session: https://claude.ai/code/session_01S9YxTH6xRfC6bq3vbgmTQD
Deleting a thread never closes the viewer's socket (it lives on the directory DO), so a mounted view kept a zombie transcript whose next frame silently resurrected the deleted thread (cloudflare/agents#2003). Switch to the most recent surviving thread before deleting; when none survives, remount the view after the delete so it reconnects to a fresh default thread. Also evict the deleted thread's transcript cache. Claude-Session: https://claude.ai/code/session_01S9YxTH6xRfC6bq3vbgmTQD
Think assembles the model prompt from the stored path, so a regenerated turn still carried the old assistant answer plus a provider-prefill "continue your previous response" instruction — regenerate behaved like continue, worst on retries of partial failed responses. Flag regenerate turns in the request body and trim the prompt back to the last user message in beforeTurn (non-continuation turns only, so tool round-trips keep their in-flight assistant). Branch storage was already correct. Workaround for cloudflare/agents#2028; remove once #2038 ships. Claude-Session: https://claude.ai/code/session_01S9YxTH6xRfC6bq3vbgmTQD
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/workspaces/components/ai-chat/useAiChatPanelController.ts">
<violation number="1" location="src/features/workspaces/components/ai-chat/useAiChatPanelController.ts:73">
P1: When deleting the sole active default thread, `selectThread(undefined)` still resolves to the same default ID, so the view stays mounted during deletion. Unmount or disable the thread view before issuing this delete, then remount a fresh default thread after success.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const survivorId = wasActive ? threads.find((thread) => thread.id !== threadId)?.id : undefined; | ||
|
|
||
| if (wasActive) { | ||
| selectThread(survivorId); |
There was a problem hiding this comment.
P1: When deleting the sole active default thread, selectThread(undefined) still resolves to the same default ID, so the view stays mounted during deletion. Unmount or disable the thread view before issuing this delete, then remount a fresh default thread after success.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/useAiChatPanelController.ts, line 73:
<comment>When deleting the sole active default thread, `selectThread(undefined)` still resolves to the same default ID, so the view stays mounted during deletion. Unmount or disable the thread view before issuing this delete, then remount a fresh default thread after success.</comment>
<file context>
@@ -59,19 +61,33 @@ export function useAiChatPanelController({ workspaceId }: UseAiChatPanelControll
+ const survivorId = wasActive ? threads.find((thread) => thread.id !== threadId)?.id : undefined;
+
+ if (wasActive) {
+ selectThread(survivorId);
+ }
+
</file context>
Cap the in-session transcript cache at 8 threads (LRU via Map order) so long sessions don't retain every visited transcript. On a failed delete, keep whatever thread the user is on instead of reselecting the errored one — it's still in the list and the toast explains. Claude-Session: https://claude.ai/code/session_01S9YxTH6xRfC6bq3vbgmTQD
|
Review triage: fixed the unbounded transcript cache (now LRU-capped at 8 threads) and removed the failed-delete reselect instead of guarding it. Not addressing: the sole-default-thread delete race needs a send within the delete RPC window and the post-delete remount heals it (root fix is upstream cloudflare/agents#2024); the carry-prompt test comment refers to code already removed; component-render tests for the new banner branches would only assert trivial conditionals — the error-state derivation they render is unit-tested in ai-chat-error-state.test.ts. |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/workspaces/components/ai-chat/useWorkspaceAiChat.ts">
<violation number="1" location="src/features/workspaces/components/ai-chat/useWorkspaceAiChat.ts:38">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
The new `cacheTranscript` LRU eviction logic is untested. Add at least a regression test asserting that the transcript cache caps at `TRANSCRIPT_CACHE_MAX_THREADS`, evicts the oldest entry when over capacity, and refreshes recency on re-insertion.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const TRANSCRIPT_CACHE_MAX_THREADS = 8; | ||
| const transcriptCache = new Map<string, AiChatMessage[]>(); | ||
|
|
||
| function cacheTranscript(threadId: string, messages: AiChatMessage[]) { |
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
The new cacheTranscript LRU eviction logic is untested. Add at least a regression test asserting that the transcript cache caps at TRANSCRIPT_CACHE_MAX_THREADS, evicts the oldest entry when over capacity, and refreshes recency on re-insertion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/useWorkspaceAiChat.ts, line 38:
<comment>The new `cacheTranscript` LRU eviction logic is untested. Add at least a regression test asserting that the transcript cache caps at `TRANSCRIPT_CACHE_MAX_THREADS`, evicts the oldest entry when over capacity, and refreshes recency on re-insertion.</comment>
<file context>
@@ -30,8 +30,22 @@ const AI_CHAT_RENDER_THROTTLE_MS = 100;
+const TRANSCRIPT_CACHE_MAX_THREADS = 8;
const transcriptCache = new Map<string, AiChatMessage[]>();
+function cacheTranscript(threadId: string, messages: AiChatMessage[]) {
+ transcriptCache.delete(threadId);
+ transcriptCache.set(threadId, messages);
</file context>
1 participant
Problem
When a chat run fails or is stopped, the UI often left the user with nothing actionable — and a deep dive into the framework (cloudflare/agents + Think source, their issue tracker, and this repo's local patches) surfaced deeper problems: deleting a thread while viewing it leaves a zombie transcript that silently resurrects the thread, switching threads always suspends on a redundant fetch, and regeneration secretly behaves like "continue" instead of a fresh answer.
Changes
Error/abort affordances
data-*parts from old transcripts.Thread lifecycle (verified against framework source)
Regeneration correctness
beforeTurn(non-continuation only). Workaround for Think regeneration includes the previous response in the model prompt cloudflare/agents#2028; remove once upstream #2038 ships.Upstream
getInitialMessages: null(blank history on mid-stream connects) and not patching Think for regeneration (app-side workaround suffices until #2038).Deliberately skipped
A deploy version-skew "refresh to update" toast remains the biggest untouched gap; it needs build/deploy plumbing that deserves its own PR.
Verification
tsc --noEmitclean; all 62 ai-chat vitest tests pass. Framework claims verified againstagents@0.19.0/@cloudflare/think@0.15.0dist (with this repo's patches applied) and the upstream repo at main.https://claude.ai/code/session_01S9YxTH6xRfC6bq3vbgmTQD