Skip to content

fix interruption for pydantic-ai chatbot#9620

Open
Light2Dark wants to merge 4 commits into
mainfrom
sham/fix-pydantic-ai-chat-interruption
Open

fix interruption for pydantic-ai chatbot#9620
Light2Dark wants to merge 4 commits into
mainfrom
sham/fix-pydantic-ai-chat-interruption

Conversation

@Light2Dark
Copy link
Copy Markdown
Collaborator

📝 Summary

Fixes #9562. Received reasoning-delta for missing reasoning part with ID "..." after Stop → resend in mo.ui.chat.

Root cause: marimo routes all chat chunks through one long-lived ReadableStream (kernel → WebSocket), so stale chunks from a stopped run can leak into a new run's stream — the SDK parser then sees a reasoning-delta for an id it never saw a reasoning-start for and throws.

Changes

Frontend (frontend/src/plugins/impl/chat/)

  • Generate a request_id per prompt; track it in activeRequestIdRef.
  • Extract chunk routing into routeIncomingChatChunk and drop chunks whose message_id doesn't match the active run.
  • On fetch abort, fire a cancel_prompt RPC to the backend.

Backend (marimo/_plugins/ui/_impl/chat/chat.py)

  • New cancel_prompt RPC; in-flight prompts run as asyncio.Tasks keyed by request_id and are cancelled cooperatively.
  • await asyncio.sleep(0) in the sync-generator loop so CancelledError can be raised at yield points.
  • On cancel, send AbortChunk and synthesize *-end for any open text / reasoning blocks so the UI parser transitions parts out of state: "streaming".
  • ChunkSerializer: single drain helper shared by on_end and close_open_blocks; cancel-path errors are logged at DEBUG instead of silently swallowed.
  • Docstring on mo.ui.chat describing how users can handled aborted chats / tool calls.

📋 Pre-Review Checklist

  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • Video or media evidence is provided for any visual changes (optional).

✅ Merge Checklist

  • I have read the contributor guidelines.
  • Documentation has been updated where applicable, including docstrings for API changes.
  • Tests have been added for the changes made.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment May 22, 2026 3:41am

Request Review

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Bundle Report

Changes will increase total bundle size by 86.54kB (0.34%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
marimo-esm 25.26MB 86.54kB (0.34%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: marimo-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/JsonOutput-*.js 189.41kB 556.64kB 51.58% ⚠️
assets/index-*.js -176.27kB 431.49kB -29.0%
assets/index-*.css 600 bytes 366.91kB 0.16%
assets/dist-*.js -81 bytes 102 bytes -44.26%
assets/dist-*.js 158 bytes 335 bytes 89.27% ⚠️
assets/dist-*.js 172 bytes 276 bytes 165.38% ⚠️
assets/dist-*.js -73 bytes 183 bytes -28.52%
assets/dist-*.js -107 bytes 169 bytes -38.77%
assets/dist-*.js -56 bytes 104 bytes -35.0%
assets/dist-*.js -6 bytes 177 bytes -3.28%
assets/dist-*.js -99 bytes 160 bytes -38.22%
assets/dist-*.js 266 bytes 403 bytes 194.16% ⚠️
assets/dist-*.js -27 bytes 137 bytes -16.46%
assets/dist-*.js -7 bytes 169 bytes -3.98%
assets/dist-*.js 79 bytes 183 bytes 75.96% ⚠️
assets/dist-*.js 33 bytes 137 bytes 31.73% ⚠️
assets/dist-*.js -5 bytes 164 bytes -2.96%
assets/dist-*.js -65 bytes 104 bytes -38.46%
assets/dist-*.js -283 bytes 104 bytes -73.13%
assets/dist-*.js 157 bytes 259 bytes 153.92% ⚠️
assets/dist-*.js 39 bytes 176 bytes 28.47% ⚠️
assets/dist-*.js 52 bytes 387 bytes 15.52% ⚠️
assets/dist-*.js -147 bytes 256 bytes -36.48%
assets/edit-*.js 78 bytes 325.17kB 0.02%
assets/reveal-*.js 72.54kB 249.59kB 40.98% ⚠️
assets/add-*.js 268 bytes 202.26kB 0.13%
assets/layout-*.js 499 bytes 198.73kB 0.25%
assets/dependency-*.js 341 bytes 156.71kB 0.22%
assets/panels-*.js 680 bytes 47.89kB 1.44%
assets/chat-*.js 595 bytes 15.26kB 4.06%
assets/react-*.browser.esm-Ce2ksurd.js (New) 25.64kB 25.64kB 100.0% 🚀
assets/state-*.js -13 bytes 24.79kB -0.05%
assets/download-*.js 5 bytes 9.48kB 0.05%
assets/scratchpad-*.js 15 bytes 8.4kB 0.18%
assets/config-*.js 315 bytes 6.41kB 5.17% ⚠️
assets/useBoolean-*.js -2.75kB 3.02kB -47.66%
assets/slides-*.css 224 bytes 529 bytes 73.44% ⚠️
assets/react-*.browser.esm-CgWOEYeG.js (Deleted) -25.64kB 0 bytes -100.0% 🗑️

Files in assets/index-*.js:

  • ./src/plugins/impl/chat/ChatPlugin.tsx → Total Size: 2.21kB

Files in assets/add-*.js:

  • ./src/components/chat/chat-utils.ts → Total Size: 4.62kB

Files in assets/chat-*.js:

  • ./src/plugins/impl/chat/chat-ui.tsx → Total Size: 26.19kB

@Light2Dark Light2Dark added the bug Something isn't working label May 20, 2026
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 6 files

Architecture diagram
sequenceDiagram
    participant UI as Chatbot (Frontend)
    participant RS as ReadableStream Controller
    participant API as Marimo Kernel (Backend)
    participant Task as Async Prompt Task
    participant AI as AI Model / Generator

    Note over UI,AI: NEW: Prompt Execution with Request ID Tracking

    UI->>UI: NEW: generateUUID() as request_id
    UI->>API: send_prompt(request_id, messages)
    API->>Task: NEW: Create task keyed by request_id
    Task->>AI: Invoke model generator
    
    loop Streaming
        AI-->>Task: yield delta
        Task->>Task: NEW: await asyncio.sleep(0) (interruption point)
        Task-->>UI: WS: stream_chunk(message_id=request_id, content)
        UI->>UI: CHANGED: routeIncomingChatChunk()
        opt NEW: message_id == activeRequestId
            UI->>RS: enqueue(content)
        end
    end

    Note over UI,AI: NEW: Interruption Flow (User clicks "Stop")

    UI->>RS: close()
    UI->>UI: activeRequestIdRef.current = null
    UI->>API: NEW: cancel_prompt(request_id)
    API->>Task: task.cancel()
    
    alt Task Interrupted
        Task->>AI: Raise CancelledError (sync/async)
        AI->>Task: Cleanup (try/finally)
        Task->>Task: NEW: serializer.close_open_blocks()
        Task-->>UI: WS: NEW: Send AbortChunk + is_final: true
    end

    Note over UI,AI: Stale Chunk Handling (Race Condition)

    UI->>UI: User starts NEW prompt (request_id=B)
    Task-->>UI: WS: Late chunk from OLD prompt (request_id=A)
    UI->>UI: NEW: Check message_id (A) != activeRequestId (B)
    UI->>UI: NEW: Drop stale chunk (prevents SDK parser error)
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread frontend/src/plugins/impl/chat/chat-ui.tsx
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a streaming interruption bug in mo.ui.chat when using pydantic-ai (Vercel AI SDK protocol), where late chunks from an aborted run could corrupt the next run’s chunk stream/state.

Changes:

  • Frontend: generate a per-prompt request_id, route incoming chunks through a stale-chunk filter keyed by the active request, and call a new cancel_prompt RPC on abort.
  • Backend: tag all streamed chunks with the request/message id, track in-flight prompt tasks by request_id, add cancel_prompt RPC, and emit protocol-consistent end/abort chunks on cancellation.
  • Tests: add backend cancellation/serializer tests and frontend stale-chunk routing tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/_plugins/ui/_impl/chat/test_chat.py Adds coverage for request_id tagging, cancellation behavior, and serializer block-drain on cancel.
marimo/_plugins/ui/_impl/chat/chat.py Implements request-scoped streaming, cooperative cancellation, and cancellation chunk emission/draining.
frontend/src/plugins/impl/chat/types.ts Adds request_id to send requests and introduces CancelPromptRequest type.
frontend/src/plugins/impl/chat/ChatPlugin.tsx Extends RPC surface with cancel_prompt and validates request_id on send_prompt.
frontend/src/plugins/impl/chat/chat-ui.tsx Generates request ids, drops stale chunks, and triggers backend cancel on Stop/abort.
frontend/src/plugins/impl/chat/__tests__/chat-ui.test.ts Adds unit tests ensuring stale chunks are dropped and active streams are not closed by stale finals.
Comments suppressed due to low confidence (1)

frontend/src/plugins/impl/chat/chat-ui.tsx:258

  • ReadableStream underlying source start(controller) return value is ignored by the platform, so the cleanup callback that removes the abort listener will never run. This leaves an abort event listener attached to signal longer than intended (and the closure retains controller/props). Use signal.addEventListener('abort', abortHandler, { once: true }), or store abortHandler/signal and remove the listener in cancel() and/or when the stream is closed.
            start(controller) {
              frontendStreamControllerRef.current = controller;
              activeRequestIdRef.current = requestId;

              const abortHandler = () => {
                // Close the local controller first so the chat status flips to
                // "ready" immediately and any racing chunks are dropped; then
                // fire-and-forget the backend cancel so the kernel stops the
                // model and we don't waste tokens / leak chunks to the next
                // run.
                try {
                  controller.close();
                } catch (error) {
                  Logger.debug("Controller may already be closed", { error });
                }
                frontendStreamControllerRef.current = null;
                activeRequestIdRef.current = null;
                void props
                  .cancel_prompt({ request_id: requestId })
                  .catch((error: Error) => {
                    Logger.debug("cancel_prompt failed", { error });
                  });
              };
              signal?.addEventListener("abort", abortHandler);

              return () => {
                signal?.removeEventListener("abort", abortHandler);
              };

Comment thread marimo/_plugins/ui/_impl/chat/chat.py Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@Light2Dark Light2Dark marked this pull request as ready for review May 20, 2026 04:24
## 📝 Summary

<!--
If this PR closes any issues, list them here by number (e.g., Closes
#123).

Detail the specific changes made in this pull request. Explain the
problem addressed and how it was resolved. If applicable, provide before
and after comparisons, screenshots, or any relevant details to help
reviewers understand the changes easily.
-->
Supports more AI sdk parts, refactors logic to be more maintainable.

- ChatMessage no longer loses unmodeled SDK fields. Approvals (and e.g.
`callProviderMetadata`, `providerExecuted`, `preliminary`) live on AI
SDK parts that marimo's typed dataclasses don't model. The message now
snapshots the raw wire payload per-part in _raw_parts, so those fields
survive every round-trip
- `pydantic_ai._build_ui_messages` uses the raw payload. It now calls
`message.raw_or_dumped_parts()` so the approval/tool state the frontend
just sent us makes it into the agent run unmodified. The old `asdict` +
`_remove_none_values` path was lossy.
- `sanitize_part` strips keys the AI SDK's { ...part, state, ... }
spread can leak from prior tool states (e.g. a stale output clinging to
an approval-requested part).
- hasPendingToolCalls (frontend) rewritten. The old predicate treated
"every tool ready & no trailing text" silently looped whenever an
assistant message ended in a non-text part (file, source-url, data-*,
reasoning) after a completed tool call. The fix uses some native AI sdk
logic and some testing.

<img width="921" height="787" alt="image"
src="https://github.com/user-attachments/assets/53f4a146-9554-4135-b9e9-459317a823dc"
/>

## 📋 Pre-Review Checklist
<!-- These checks need to be completed before a PR is reviewed -->

- [x] For large changes, or changes that affect the public API: this
change was discussed or approved through an issue, on
[Discord](https://marimo.io/discord?ref=pr), or the community
[discussions](https://github.com/marimo-team/marimo/discussions) (Please
provide a link if applicable).
- [x] Any AI generated code has been reviewed line-by-line by the human
PR author, who stands by it.
- [x] Video or media evidence is provided for any visual changes
(optional). <!-- PR is more likely to be merged if evidence is provided
for changes made -->

## ✅ Merge Checklist

- [x] I have read the [contributor
guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md).
- [ ] Documentation has been updated where applicable, including
docstrings for API changes.
- [x] Tests have been added for the changes made.

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mo.ui.chat + pydantic-ai: cancelling a streaming response mid-reasoning corrupts Vercel AI SDK protocol state for subsequent prompts

2 participants