Skip to content

fix(desktop): preserve streamed text when provider sends empty completion - #41922

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/desktop-empty-completion-text
Open

liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/desktop-empty-completion-text

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Preserves streamed assistant text when a provider sends a message.complete event with empty text/rendered fields. This fixes a bug where responses from NVIDIA NIM (and any provider that sends finish_reason: "stop" with no content in the final chunk) would flash briefly and then disappear from the Desktop UI.

Related Issue

Fixes #41898

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • apps/desktop/src/app/session/hooks/use-message-stream.ts:
    • replaceTextPart: early-return existing parts when finalText is empty, preserving streamed text accumulated via message.delta events
    • shouldHydrate: skip stored-session hydration when streamed text already exists but finalText is empty, preventing stale snapshot from overwriting visible response

How to Test

  1. Configure a provider that sends empty content in the final message.complete chunk (e.g. NVIDIA NIM with nvidia/nemotron-3-ultra-550b-a55b)
  2. Launch Hermes Desktop App
  3. Send any prompt
  4. Verify the assistant response remains visible after streaming completes (no flash-and-disappear)
  5. Test with a normal provider (e.g. DeepSeek) to verify no regression — responses should still appear and stay
  6. Test interruption: start a response, cancel mid-stream, verify partial text is preserved

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: use-message-stream.tsreplaceTextPart, completeAssistantMessage, shouldHydrate
  • Callers: message.complete event handler (line 803), streaming delta handlers
  • Blast radius: LOW — scoped to Desktop app's message completion path
  • Related patterns: hydrateFromStoredSession fallback (line 561), completeMessage (line 491)

…tion

When a provider (e.g. NVIDIA NIM) sends a message.complete event with
empty text/rendered fields after streaming content via message.delta,
the replaceTextPart helper stripped all accumulated text parts without
adding anything back — causing the rendered response to vanish.

Fix: early-return the existing parts when finalText is empty, so streamed
content is preserved.  Also adjust shouldHydrate to skip the stored-
session hydration fallback when streamed text already exists, preventing
a stale snapshot from overwriting the visible response.

Fixes NousResearch#41898

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for isolating the empty-completion path. The defect remains on current main: gateway-event.ts:334-335 passes an empty completion string through, while index.ts:363-377 removes all streamed text parts and adds none when the final text is empty. index.ts:432-457 then enables the hydration fallback for that same empty completion.

Problems

  • The PR diff changes no tests. Current hook tests under apps/desktop/src/app/session/hooks/use-message-stream/ cover todo cleanup and utilities, not an empty message.complete after streamed deltas.

Suggested changes

  • Salvage the preservation guard into apps/desktop/src/app/session/hooks/use-message-stream/index.ts, which replaced the old target in behavior-preserving refactor 086343854dbf723acfc529d2580c260cc3713d0b.
  • Add a regression test for message.delta text followed by an empty message.complete, including the hydration outcome.

Automated hermes-sweeper review.

// sends finish_reason:"stop" with no content), preserve the
// streamed text parts that were accumulated via message.delta.
// Otherwise the already-rendered response vanishes on completion.
if (!finalText) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please add a Desktop regression test for streamed message.delta text followed by an empty message.complete. This branch is the fix's essential behavior, and the PR currently changes no test files.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/streaming Streaming responses: gateway delivery, provider wire labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/streaming Streaming responses: gateway delivery, provider wire P3 Low — cosmetic, nice to have provider/nvidia NVIDIA NIM sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Desktop app — assistant response flashes and disappears immediately (provider: NVIDIA NIM)

3 participants