Skip to content

fix: emit reasoning events in AGUI interface - #7429

Merged
Mustafa-Esoofally merged 5 commits into
mainfrom
worktree-fix-agui-reasoning
Apr 9, 2026
Merged

Mustafa-Esoofally merged 5 commits into
mainfrom
worktree-fix-agui-reasoning

Conversation

@Mustafa-Esoofally

@Mustafa-Esoofally Mustafa-Esoofally commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Map Agno reasoning events to AG-UI protocol reasoning event types (REASONING_START, REASONING_MESSAGE_CONTENT, etc.) using isinstance() dispatch for type-safe Agent and Team event handling
  • Handle two distinct reasoning producers:
    • Native model reasoning (o4-mini, Claude extended thinking) → ReasoningContentDeltaEvent with true streaming deltas
    • ReasoningTools (think/analyze tool calls) → ReasoningStepEvent with structured ReasoningStep content, formatted as text delta per step (avoids accumulated content duplication)
  • Emit fallback REASONING_START/REASONING_MESSAGE_START when content arrives without a preceding reasoning_started event
  • Close orphaned reasoning sessions on stream completion/abort (including synthetic completion path)
  • Remove redundant STEP_STARTED/STEP_FINISHED wrapping — replaced by dedicated REASONING_* events (ag-ui-protocol ≥0.1.15)
  • Bump ag-ui-protocol dependency to >=0.1.15 for reasoning event types

Why Two Reasoning Paths?

Agno has two distinct reasoning producers that emit different event types:

Producer Event Type reasoning_content field Correct mapping
Native model (o4-mini, Claude extended thinking) ReasoningContentDeltaEvent True streaming delta (new tokens only) Use directly as REASONING_MESSAGE_CONTENT.delta
ReasoningTools (think/analyze tool calls) ReasoningStepEvent Accumulated (all previous steps + new step) Format chunk.content (single ReasoningStep) as delta text

The AG-UI SDK concatenates deltas: targetMessage.content += delta (source). Sending accumulated content as delta duplicates text.

Bugs Fixed

  1. Accumulated content duplicationReasoningStepEvent.reasoning_content is accumulated (all steps so far), but AG-UI SDK concatenates deltas. Sending accumulated content as delta caused step 1 to appear multiple times. Fixed by formatting chunk.content (the single ReasoningStep object) directly via _format_reasoning_step_delta().
  2. Orphaned reasoning sessions — Synthetic completion path (stream ends without run_completed) didn't pass reasoning_message_id, leaving REASONING_START unclosed. Fixed by threading reasoning_message_id through both sync and async completion paths.
  3. Protocol violationREASONING_MESSAGE_CONTENT emitted without preceding REASONING_START/REASONING_MESSAGE_START when reasoning_content_delta arrived before reasoning_started. Fixed by emitting START events in fallback path.
  4. Redundant STEP wrappingSTEP_STARTED/STEP_FINISHED emitted alongside REASONING_* events. Removed since dedicated reasoning events replace the generic step markers.

Test Matrix

All scenarios tested on both AG-UI Dojo and os.agno.com:

# Scenario Platform Query Result
1 o4-mini native reasoning AG-UI Dojo "What is 15*37?" PASS — "Thought for 5 seconds", step-by-step response
2 ReasoningTools simple AG-UI Dojo "What is 15*37?" PASS — "Thought for 2 seconds", single step, no duplication
3 ReasoningTools complex AG-UI Dojo Fox/chicken/grain puzzle PASS — "Thought for 4 seconds", 3 reasoning steps, no duplication
4 o4-mini native reasoning os.agno.com "What is 15*37?" PASS — "REASONING > 555", "Worked for 5 s"
5 ReasoningTools simple os.agno.com "What is 15*37?" PASS — "3 TOOLS CALLED", "REASONING", "Worked for 9 s"
6 ReasoningTools complex os.agno.com Fox/chicken/grain puzzle PASS — "3 TOOLS CALLED", "REASONING", "Worked for 11 s", multi-step solution

Raw SSE Event Sequence (ReasoningTools, complex query)

REASONING_START → REASONING_MESSAGE_START
  → REASONING_MESSAGE_CONTENT(delta="## Solving the river crossing problem\n...")
  → REASONING_MESSAGE_CONTENT(delta="## Developing step-by-step solution\n1. Take the chicken...")
  → REASONING_MESSAGE_CONTENT(delta="## Analysis of the proposed crossing steps\n...")
→ REASONING_MESSAGE_END → REASONING_END
→ TEXT_MESSAGE_START → TEXT_MESSAGE_CONTENT... → TEXT_MESSAGE_END
→ RUN_FINISHED

Each step appears exactly once — no accumulated duplication.

Type of change

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

Checklist

  • Formatted using ./scripts/format.sh
  • Validated using ./scripts/validate.sh
  • 33/33 existing AGUI unit tests pass
  • All 4 bugs reproduced with targeted scripts, then verified fixed (5/5 pass)
  • Verified via curl: full REASONING_* event sequence for both native reasoning and ReasoningTools
  • Verified via AG-UI Dojo: "Thought for N seconds" rendered correctly for both reasoning paths
  • Verified on os.agno.com: Feature parity with AgentOS native streaming for both reasoning paths
  • Complex multi-step query (fox/chicken/grain puzzle) — 3 reasoning steps, no duplication
  • AG-UI SDK source confirmed: targetMessage.content += delta (concatenation), validating delta approach
  • Codex review: confirmed isinstance dispatch, fallback START emission, and completion cleanup are correct

Test plan

  • Run pytest libs/agno/tests/unit/app/test_agui_app.py — 33 pass
  • Start reasoning agent (o4-mini + ReasoningTools), curl /agui endpoint, verify REASONING_* events in SSE stream
  • Run AG-UI Dojo locally, select Agno > Agentic Chat + Backend Tool Rendering, confirm "Thought for N seconds" renders with expandable reasoning content
  • Test on os.agno.com with both Thinking Agent and Reasoning Tools Agent — feature parity confirmed

Fixes #7216

Map Agno reasoning events to AG-UI protocol reasoning event types
instead of generic STEP_STARTED/STEP_FINISHED. This enables CopilotKit
frontends to render "Thought for N seconds" with expandable reasoning
content.

- Emit REASONING_START, REASONING_MESSAGE_START, REASONING_MESSAGE_CONTENT,
  REASONING_MESSAGE_END, and REASONING_END events during reasoning
- Handle both Agent (RunEvent) and Team (TeamRunEvent) reasoning events
- Close orphaned reasoning sessions on stream completion/abort
- Bump ag-ui-protocol dependency to >=0.1.15 for reasoning event types

Fixes #7216
@Mustafa-Esoofally
Mustafa-Esoofally requested a review from a team as a code owner April 8, 2026 19:16
@Mustafa-Esoofally
Mustafa-Esoofally force-pushed the worktree-fix-agui-reasoning branch 7 times, most recently from ab03e61 to 57d3b9e Compare April 8, 2026 22:09
- Use isinstance() dispatch for Agent/Team reasoning event types
- Handle ReasoningContentDeltaEvent (native model reasoning) as true streaming delta
- Handle ReasoningStepEvent (ReasoningTools) by formatting chunk.content (single step)
  instead of using accumulated reasoning_content field which duplicated text
- Emit REASONING_START/REASONING_MESSAGE_START in fallback when content arrives
  without preceding reasoning_started event
- Pass reasoning_message_id through synthetic completion path to close orphaned sessions
- Remove redundant STEP_STARTED/STEP_FINISHED wrapping (replaced by REASONING_* events)
- Bump ag-ui-protocol dependency to >=0.1.15 for reasoning event types
- Update test to verify REASONING_* event types and ordering
@Mustafa-Esoofally
Mustafa-Esoofally force-pushed the worktree-fix-agui-reasoning branch from 57d3b9e to 22de607 Compare April 8, 2026 22:14
@ysolanky

ysolanky commented Apr 9, 2026

Copy link
Copy Markdown
Member

PR Review: fix: emit reasoning events in AGUI interface

Overall: Good PR. The approach is sound - mapping Agno reasoning events to AG-UI REASONING_* protocol events instead of generic STEP_STARTED/STEP_FINISHED. The two-path handling (native model vs ReasoningTools) is well-motivated.

Issues

1. Potential dispatch ordering bug (Medium)

The _create_events_from_chunk function uses an elif chain. The first branches check chunk.event == RunEvent.run_content. But ReasoningContentDeltaEvent and ReasoningStepEvent also have a .event field (reasoning_content_delta and reasoning_step), so they won't match run_content and will fall through to the isinstance checks correctly. However, there's a subtlety: ReasoningStartedEvent has event = RunEvent.reasoning_started.value, and the old code checked chunk.event == RunEvent.reasoning_started (comparing string to enum). The new code uses isinstance() which is strictly better and avoids this string/enum comparison ambiguity. ✓

2. Duplicate START event emission pattern (Low - style)

The fallback ensure_reasoning_started() + emit START events pattern is duplicated 3 times (in ReasoningContentDeltaEvent, ReasoningStepEvent, and ReasoningStartedEvent handlers). Consider extracting a helper like _emit_reasoning_start_if_needed(event_buffer) that returns the list of START events. Not a blocker but reduces repetition.

3. Missing test coverage for ReasoningStepEvent path (Medium)

The test only covers ReasoningContentDeltaEvent (native model reasoning). There's no test for the ReasoningStepEvent path (ReasoningTools), which has the more complex formatting logic via _format_reasoning_step_delta(). This is the path that had the accumulated content duplication bug - it deserves a dedicated test.

4. Missing test for orphaned reasoning cleanup (Low)

The _create_completion_events now closes orphaned reasoning sessions, but there's no test where a stream ends mid-reasoning (without ReasoningCompletedEvent). A test that yields ReasoningStartedEvent + ReasoningContentDeltaEvent then RunCompletedEvent (skipping ReasoningCompletedEvent) would verify the cleanup path.

5. _format_reasoning_step_delta always appends \n\n (Low)

The function returns "\n".join(parts) + "\n\n" if parts else "". Since parts always has at least the title ("## Thinking" if step.title is None), the if parts check is redundant and the trailing \n\n always gets appended. This is fine for rendering but worth noting.

What looks good

  • isinstance() dispatch is the right pattern - more robust than string comparison on .event
  • EventBuffer extensions (start_reasoning, ensure_reasoning_started, end_reasoning) are clean
  • Orphaned session cleanup in _create_completion_events is important for protocol correctness
  • PR description is thorough with clear test matrix

Verdict

The implementation is correct and well-structured. The main ask would be adding a test for the ReasoningStepEvent path since that's where the duplication bug was, and optionally a test for orphaned reasoning cleanup. The rest are minor style nits.

Mustafa-Esoofally and others added 2 commits April 9, 2026 08:06
…oning

- Test ReasoningStepEvent path with 2 steps: verifies no accumulated
  content duplication, each delta contains only its own step, and
  step numbers are present
- Test orphaned reasoning cleanup: verifies REASONING_END is emitted
  when stream ends without ReasoningCompletedEvent
@Mustafa-Esoofally

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review! Here's how we addressed each point:

1. Dispatch ordering (Medium) — Verified: all 4 reasoning event types have unique .event values (ReasoningStarted, ReasoningStep, etc.) that don't match any earlier elif branch (RunContent, ToolCallStarted, etc.). The isinstance() dispatch is strictly better than the old string/enum comparison. No issue here. ✓

2. Duplicate START emission pattern (Low/Style) — Considered extracting a helper, but the 3 branches have different control flow around the same 2-line append:

Branch 1 (Started):  unconditional start → append START events
Branch 2 (ContentDelta): conditional start (ensure_reasoning_started) → append START if new → emit delta
Branch 3 (StepEvent): conditional start → append START if new → next_reasoning_step() → format step → emit

A helper would only factor out 2 append calls but each branch wraps them differently (unconditional vs if is_new) and has unique logic after. Skipping to avoid premature abstraction.

3. Missing ReasoningStepEvent test (Medium) — Added test_reasoning_step_events_no_duplication: yields 2 ReasoningStepEvents with cumulative reasoning_content, verifies each delta contains only its own step (no duplication), and checks step numbers are present. ✓

4. Missing orphaned cleanup test (Low) — Added test_orphaned_reasoning_cleanup: stream ends after ReasoningStartedEvent + one delta without ReasoningCompletedEvent, verifies REASONING_END and RUN_FINISHED are still emitted. ✓

5. Trailing \n\n (Low) — Intentional for step separation in the AG-UI stream. Skipping.

35/35 tests passing, all CI green.

@Mustafa-Esoofally
Mustafa-Esoofally merged commit eefce36 into main Apr 9, 2026
5 checks passed
@Mustafa-Esoofally
Mustafa-Esoofally deleted the worktree-fix-agui-reasoning branch April 9, 2026 16:58
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.

[Bug] AGUI integration does not emit reasoning events

3 participants