Skip to content

Fix local CLI streamed generation error handling#7135

Open
Lyxot wants to merge 3 commits into
unslothai:mainfrom
Lyxot:fix/cli-local-stream-errors
Open

Fix local CLI streamed generation error handling#7135
Lyxot wants to merge 3 commits into
unslothai:mainfrom
Lyxot:fix/cli-local-stream-errors

Conversation

@Lyxot

@Lyxot Lyxot commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Apply the existing typed streamed-error adapter to local unsloth inference and unsloth chat streams, not only MLX-distributed streams.
  • Preserve ordinary model completions, including plain text beginning with Error:.
  • Add focused local CLI regression coverage for output rendering, chat history rollback, normal completions, and backend cleanup.

Root cause

GenStreamError deliberately subclasses str, allowing existing stream display and collection consumers to handle cumulative text uniformly. The CLI commands only wrapped streams with raise_on_streamed_error in MLX-distributed mode, so local display helpers accepted a typed backend failure as ordinary assistant output.

The fix keeps error classification type-based and moves no logic into the rendering helpers: both command stream boundaries now consistently apply the existing adapter.

User impact

Local unsloth inference now fails instead of printing a backend generation error as the completion. Interactive unsloth chat reports the error, removes the unanswered user turn, and continues without storing the failure as assistant history.

Validation

python -m pytest unsloth_cli/tests/test_inference_chat.py -q

Result: 46 passed.

Copilot AI review requested due to automatic review settings July 15, 2026 05:13

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request simplifies stream error handling by always invoking raise_on_streamed_error instead of conditionally calling it only when is_mlx_distributed is true. It also updates related docstrings and comments to reflect this change and adds new unit tests (test_inference_local_handles_stream and test_chat_local_handles_stream) to verify stream error handling in local environments. There are no review comments, so I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 26b9622927

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread unsloth_cli/commands/inference.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 local CLI streamed-generation error handling by applying the existing typed streamed-error adapter (raise_on_streamed_error) to local unsloth inference and unsloth chat streams (previously only applied in MLX-distributed paths). This ensures backend failures carried via GenStreamError are raised/handled as errors instead of being rendered/stored as normal assistant output, while preserving ordinary model text that happens to begin with Error:.

Changes:

  • Wrap local inference streams with raise_on_streamed_error so typed backend failures raise (instead of printing as completion text).
  • Wrap local chat generation streams with raise_on_streamed_error so chat rolls back the failed user turn and continues.
  • Add regression tests covering local inference/chat output rendering, rollback behavior, normal completions beginning with Error:, and backend cleanup.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
unsloth_cli/commands/inference.py Applies raise_on_streamed_error to inference streams in all modes so local runs correctly raise on typed backend errors.
unsloth_cli/commands/chat.py Applies raise_on_streamed_error to chat generation streams so local chat handles typed backend failures consistently.
unsloth_cli/_inference.py Clarifies adapter intent in comments for type-based streamed error detection.
studio/backend/core/inference/orchestrator.py Tightens GenStreamError docstring to reflect broader (non-distributed-only) consumption.
unsloth_cli/tests/test_inference_chat.py Adds focused regression tests for local inference/chat streamed error handling and history rollback.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants