Skip to content

fix(desktop): accumulate MoA reference reasoning blocks instead of replacing - #64689

Closed
wesleysimplicio wants to merge 1 commit into
NousResearch:mainfrom
wesleysimplicio:fix/moa-reference-reasoning-accumulate
Closed

fix(desktop): accumulate MoA reference reasoning blocks instead of replacing#64689
wesleysimplicio wants to merge 1 commit into
NousResearch:mainfrom
wesleysimplicio:fix/moa-reference-reasoning-accumulate

Conversation

@wesleysimplicio

Copy link
Copy Markdown
Contributor

What does this PR do?

Hermes Desktop shows Mixture-of-Agents (MoA) reference-model output as labelled reasoning blocks
in the "Thinking" disclosure, one per reference model, before the aggregator's final answer
(behavior introduced in #53855). Every moa.reference gateway event, however, called
appendReasoningDelta(sessionId, text, true)replace=true — which wipes all existing
reasoning-type message parts and replaces them with exactly one new part. With two or more
reference models, each later moa.reference event erased the reasoning block built by the
earlier one(s), so only the last reference's output ever stayed visible instead of accumulating
one labelled block per reference.

The fix keeps replace=true only for the first reference (index <= 1, or missing — this
preserves the original intent of clearing any stale reasoning left over from before this turn's
references start) and switches every later reference to the existing "queue then flush
immediately" path instead, so it appends onto the already-shown blocks rather than replacing
them. Each reference arrives as one complete text block (not incremental tokens), so applying it
via an immediate flush rather than the streamed/batched queue is correct and matches how a
sibling event elsewhere in this file already applies non-streamed content immediately.

I verified the "in-flight reasoning.delta could interleave with the reference-accumulation flush"
concern raised in review by reading agent/moa_loop.py: MoAChatCompletions.reference_callback
fires "moa.reference" once per reference's already-complete text — there is no concurrent
token stream during the reference-gathering phase for the accumulation path to collide with.

How it works

flowchart TD
    A["moa.reference event (index, count, label, text)"] --> B{"index <= 1?"}
    B -->|"yes (first reference)"| C["appendReasoningDelta(replace=true)\nclears stale reasoning, seeds one fresh block\n(unchanged from before)"]
    B -->|"no (later reference)"| D["appendReasoningDelta(replace=false) queues the block,\nthen flushQueuedDeltas() applies it immediately\n→ appends after the prior reference block(s)"]
    C --> E["◇ Reference 1/2 — model-a\nadvice-a"]
    D --> F["◇ Reference 1/2 — model-a\nadvice-a\n\n◇ Reference 2/2 — model-b\nadvice-b"]
Loading

Related Issue

Fixes #64658

Type of Change

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

Changes Made

  • apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts: moa.reference handler now branches on index: <= 1 (or missing) keeps the original replace=true call; every later reference calls appendReasoningDelta(..., false) followed immediately by flushQueuedDeltas(sessionId) so it accumulates instead of replacing.
  • apps/desktop/src/app/session/hooks/use-message-stream/moa-reference-event.test.tsx: new test file (following the existing compaction-event.test.tsx pattern of rendering the real useMessageStream hook and driving handleGatewayEvent directly) — covers 2-reference accumulation (the reported repro), a single-reference (count=1) turn for regression safety, and 3+ references landing in order.

Step-by-step

  1. Traced appendReasoningDelta's replace=true branch (use-message-stream/index.ts) to confirm it filters out every existing reasoning-type part before seeding one new part — the exact mechanism the issue describes.
  2. Wrote a test reproducing the reported repro (two sequential moa.reference events); confirmed it fails against the pre-fix code (only the second reference's text survives).
  3. Changed the handler to only replace on the first reference, accumulate on every later one via the already-available flushQueuedDeltas dependency (no new API surface needed).
  4. Verified the queue-then-flush approach applies synchronously and deterministically (no await between appendReasoningDelta(..., false) and flushQueuedDeltas, so there's no window for the scheduled rAF/setTimeout flush to double-apply).
  5. Investigated (via an adversarial review pass) whether an in-flight reasoning.delta could land in the same shared per-session queue bucket as a moa.reference accumulation flush, producing unseparated/garbled text — confirmed via agent/moa_loop.py that the reference-gathering phase has no concurrent token stream, so this isn't reachable.

Acceptance Criteria

  • Given two or more MoA reference-model events in one turn, when each arrives, then every reference's labelled block remains visible afterward (none are wiped by a later one).
  • Given a single-reference (count=1) MoA turn, when the reference arrives, then it still renders correctly (no regression for the common case that motivated the original replace=true).
  • Adjacent behavior unchanged: reasoning.delta (streaming) and reasoning.available (full-replace) call sites and their existing tests are untouched and still pass.
  • Test suite passes locally with the new tests included.

How to Test

  1. On main, configure an MoA preset with 2+ reference models, run /moa <prompt>, and watch the Thinking disclosure after the second moa.reference event lands — only the latest reference's block is visible.
  2. Check out this branch.
  3. Same flow — every reference's labelled block accumulates in order.

Tests Performed

Check Command Result
New MoA reference test file npx vitest run src/app/session/hooks/use-message-stream/moa-reference-event.test.tsx (from apps/desktop) 3 passed
Full use-message-stream suite npx vitest run src/app/session/hooks/use-message-stream/ (from apps/desktop) 5 test files, 19 passed
Prettier npx prettier --check on both changed files ✅ new test file clean; the touched section of gateway-event.ts matches Prettier's output (two unrelated pre-existing lines elsewhere in the file are not Prettier-clean on main already and were intentionally left untouched to keep this diff scoped)
TypeScript npx tsc --noEmit -p tsconfig.json, filtered to touched files ✅ no errors
Fail-before/pass-after New tests run against pre-fix code ❌ 2 of 3 fail (expected '...model-b...' to contain 'model-a'); all 3 pass after the fix

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run the tests and all pass
  • I've added tests for my changes
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • Relevant documentation updated — N/A (internal event-handling detail, no user-facing docs describe it)
  • cli-config.yaml.example updated if config keys changed — N/A
  • CONTRIBUTING.md/AGENTS.md updated if architecture/workflow changed — N/A
  • Cross-platform impact considered (Windows, macOS) — Desktop UI logic, platform-agnostic; no OS-specific code touched
  • Tool descriptions/schemas updated if tool behavior changed — N/A

Screenshots / Logs

$ npx vitest run src/app/session/hooks/use-message-stream/
 Test Files  5 passed (5)
      Tests  19 passed (19)

…placing

Every moa.reference event called appendReasoningDelta(..., replace=true),
which wipes ALL existing reasoning-type message parts and seeds exactly one
new part. With two or more MoA reference models, each later reference
erased the reasoning disclosure built by earlier references, so only the
last advisor's output ever stayed visible instead of one labelled block per
reference (contradicting the multi-reference visibility behavior from
NousResearch#53855).

Only the first reference (index <= 1, or missing) now replaces — preserving
the original "clear stale reasoning from before this turn" behavior. Every
later reference accumulates via the existing queue-then-flush path instead,
applied immediately since each reference arrives as one complete block
rather than incremental tokens.

Fixes NousResearch#64658
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have labels Jul 15, 2026

@tonydwb tonydwb 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.

Code Review Summary

Verdict: Approved

Overview

Desktop fix: accumulates MoA (Mixture of Agents) reference reasoning blocks instead of replacing them. 132 additions.

Assessment

  • Correctness: Accumulating rather than replacing reference blocks preserves reasoning context — correct behavior.
  • Security: No security changes.
  • Debug artifacts: None.

Summary

Clean fix. LGTM.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. Current origin/main still routes every moa.reference event through appendReasoningDelta(..., true) at apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts:350; the replacement implementation removes all existing reasoning parts at apps/desktop/src/app/session/hooks/use-message-stream/index.ts:266-281.

The proposed first-reference replacement followed by queue-and-immediate-flush for later references matches the backend lifecycle: agent/moa_loop.py:1002-1057 collects references, preserves their configured order, emits each reference block, and only then starts aggregation. The added hook-level regression coverage exercises the affected event handler without adding new surface area.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via cluster PR #70283 (commit 385a065) — your commit cherry-picked with authorship preserved. Fixes #64658. Thanks for the clean append-not-replace fix and the hook-level test!

@teknium1 teknium1 closed this Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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 MoA reference events replace earlier advisor output instead of accumulating

4 participants