Skip to content

fix(soul): clear partial UI output when LLM step is retried - #2177

Merged
RealKai42 merged 7 commits into
MoonshotAI:mainfrom
7Sageer:fix/step-retry-timing
May 9, 2026
Merged

fix(soul): clear partial UI output when LLM step is retried#2177
RealKai42 merged 7 commits into
MoonshotAI:mainfrom
7Sageer:fix/step-retry-timing

Conversation

@7Sageer

@7Sageer 7Sageer commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Related Issue

N/A — small targeted bug fix.

Description

When a streamed LLM call fails after emitting partial content, tenacity retries the step but the aborted attempt's text/think/tool-call output was already shown to the user, so the new attempt appeared concatenated with the failed one.

This PR adds a StepRetry wire event emitted from tenacity's before_sleep hook so UI consumers can discard the partial state and render a retry banner before the new attempt's output.

  • Live shell view clears _current_content_block, _tool_call_blocks, _last_tool_call_block, then prints Retrying after <reason> · attempt N/M · <wait>. State that is independent of the LLM stream (status, compaction, MCP loading, notifications, approval queues) is intentionally preserved — see the docstring on _LiveView.discard_retry_attempt for the rationale.
  • Print backends (JsonPrinter, FinalOnlyTextPrinter, FinalOnlyJsonPrinter) drop the partial assistant-message buffers; pending notifications captured during the failed attempt still flush so they aren't lost.
  • Web client (web/src/hooks/wireTypes.ts, useSessionStream.ts) consumes StepRetry for both the parent agent and subagents: the partial thinking/text blocks and any in-flight tool-call messages from the failed attempt are removed, a single live status line Retrying after … · attempt N/M · <wait> is rendered (and updated in place across consecutive retries), and tool calls from earlier successful steps in the same turn are preserved (filtered by tc.result === undefined).
  • ACP session receives the event but cannot un-send streamed chunks; addressing the resulting visible duplication is left as a follow-up.

Wire protocol version is bumped 1.9 → 1.10 to record the additive StepRetry event, matching the convention from prior wire-type changes (#1601, #1552, #1743). The change is backward-compatible — older readers of wire.jsonl skip unknown record types via existing exception handling in WireMessageEnvelope.to_wire_message() and the replay loop, so no client breaks. Public wire docs (docs/{en,zh}/customization/wire-mode.md) are updated with the new event schema.

Tests

  • test_kimisoul_retry_recovery.py::test_step_retry_event_after_partial_stream — partial stream → 429 → retry yields only the new attempt in history.
  • test_wire_message.py — StepRetry serde snapshot.
  • test_empty_think_part_indicator.py::test_step_retry_clears_partial_content_and_updates_live_status — live view clears blocks and renders banner.
  • test_print_notifications.py — three new tests covering each print backend's discard behavior.
  • Manual smoke: triggered real 429 / connection error and verified the retry banner shows and prior partial output is not duplicated.
  • Bonus unintentional smoke: while running make gen-changelog for this PR's own version-bump commit, the upstream model returned 503 — the new retry banner rendered correctly (Retrying after server error · attempt 2/3 · <1s), confirming error_type → reason mapping and attempt/wait formatting on a real failure path.

Known limitations / follow-ups

  • ACP clients still see duplicate content on retry (chunks already streamed cannot be un-sent).
  • vis (vis/src/features/wire-viewer/*) TypeScript client does not yet handle StepRetry; it will silently ignore the event, so the wire-viewer will continue to render the failed attempt's partial output until a follow-up PR adds the handler. Web is handled in this PR (see the bullet above).
  • _run_with_connection_recovery performs first-layer retries (OAuth refresh, single-shot connection recovery) silently, without emitting StepRetry.
  • No track("api_retry", ...) telemetry and no retries counter in the session-stats endpoint yet.
  • Tools may execute during streaming, so a retry can re-invoke the same tool — pre-existing kosong behavior.

Before:
截屏2026-05-07 22 15 33

After:
截屏2026-05-07 22 17 19

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked the related issue, if any.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have run make gen-changelog to update the changelog.
  • I have run make gen-docs to update the user documentation.

Open in Devin Review

7Hanrui added 2 commits May 7, 2026 20:55
When a streamed LLM call fails after emitting partial content, tenacity
retries the step but the aborted attempt's text/think/tool-call output
was already shown to the user, so the new attempt appeared concatenated
with the failed one. Add a StepRetry wire event emitted from tenacity's
before_sleep hook so UI consumers (live shell view, print backends) can
discard the partial state and show a retry banner before the new
attempt's output.
Copilot AI review requested due to automatic review settings May 7, 2026 13:17

@devin-ai-integration devin-ai-integration 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

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

Adds a new StepRetry wire event to support clearing/discarding partial streamed UI output when an LLM step is retried (e.g., after a 429), preventing the next attempt’s stream from appearing concatenated with the failed attempt in shell/print outputs.

Changes:

  • Introduces StepRetry as a first-class wire message type and emits it from the step-level tenacity retry hook.
  • Updates shell live view and print backends to discard partial assistant/tool-call state on retry and render a retry banner (shell).
  • Adds targeted test coverage for serde, shell rendering behavior, print backend buffering behavior, and end-to-end retry recovery.

Reviewed changes

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

Show a summary per file
File Description
src/kimi_cli/wire/types.py Adds the StepRetry message and wires it into the event/message unions and exports.
src/kimi_cli/soul/kimisoul.py Emits StepRetry from tenacity’s before_sleep hook during step retries.
src/kimi_cli/ui/shell/visualize/_live_view.py Handles StepRetry by discarding partial streamed state and printing a retry banner.
src/kimi_cli/ui/print/visualize.py Drops buffered assistant output/tool-calls on StepRetry across print backends while preserving notification flushing semantics.
src/kimi_cli/acp/session.py Accepts/ignores StepRetry during ACP streaming to avoid disrupting the session loop.
tests/core/test_wire_message.py Adds serde snapshot coverage for StepRetry.
tests/core/test_kimisoul_retry_recovery.py Adds an integration-style test ensuring partial stream + retry yields correct UI/history behavior and emits StepRetry.
tests/ui_and_conv/test_empty_think_part_indicator.py Verifies live view clears partial content blocks and renders a retry banner on StepRetry.
tests/ui_and_conv/test_print_notifications.py Adds tests ensuring print backends discard partial assistant buffers on StepRetry.
CHANGELOG.md Notes the user-facing retry/partial-output clearing behavior change.
docs/en/release-notes/changelog.md Mirrors the changelog entry in the English docs.
docs/zh/release-notes/changelog.md Adds the corresponding Chinese changelog entry.

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

7Hanrui added 2 commits May 8, 2026 17:00
Update wire-mode.md (en/zh) to add the StepRetry event under the
Event union with full field documentation, and refresh the
protocol version reference. Update kimi-info.md examples to show
the bumped wire protocol version.
7Hanrui added 2 commits May 9, 2026 11:50
Replace persistent console.print of StepRetry with a live-status block
that updates in place, so consecutive retries no longer pile up as
separate lines. Cleared once new content (think/text/tool call) starts
or the turn ends.

Also tighten the surrounding docstrings: discard_retry_attempt no longer
claims the banner is a permanent scrollback boundary, and
compose_agent_output's layout section is rewritten to distinguish modal
preemption from additive composition rather than implying a strict
priority order.
@7Sageer

7Sageer commented May 9, 2026

Copy link
Copy Markdown
Collaborator Author

@codex

@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: a5885471dc

ℹ️ 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 web/src/hooks/useSessionStream.ts
discardRetryAttemptMessages was iterating over every entry in
currentToolCallsRef and clearing the map. Because that ref is session-
scoped (only ever cleared in resetState), tool calls from earlier
successful steps in the same turn would be deleted from the UI when
StepRetry fired during a later step. Filter to tool calls without a
result (the in-flight ones) and delete only those, mirroring the
running-status filter the subagent path already uses.
@7Sageer

7Sageer commented May 9, 2026

Copy link
Copy Markdown
Collaborator Author

@codex

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

ℹ️ 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".

@RealKai42
RealKai42 merged commit 3749c83 into MoonshotAI:main May 9, 2026
7Sageer added a commit to 7Sageer/kimi-cli that referenced this pull request May 9, 2026
PR MoonshotAI#2177 bumped `WIRE_PROTOCOL_VERSION` from 1.9 to 1.10 in
`src/kimi_cli/wire/protocol.py` but didn't update the e2e tests, so
`tests_e2e/test_wire_protocol.py::{test_initialize_handshake,
test_initialize_external_tool_conflict}` started failing on push to
main.

Update the three `"1.9"` literals in the e2e snapshot to `"1.10"` to
match the new server response.
RealKai42 pushed a commit that referenced this pull request May 9, 2026
@7Sageer
7Sageer deleted the fix/step-retry-timing branch May 9, 2026 12:51
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.

4 participants