feat(streaming): emit ReasoningDeltaEvent for reasoning/thinking deltas (#825)#3
feat(streaming): emit ReasoningDeltaEvent for reasoning/thinking deltas (#825)#3adityasingh2400 wants to merge 348 commits into
Conversation
|
Important Review skippedToo many files! This PR contains 299 files, which is 149 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (299)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA new Changes
Sequence DiagramsequenceDiagram
participant Client
participant Runner
participant Model
participant StreamLoop
participant EventQueue
Client->>Runner: run_streamed()
Runner->>Model: request reasoning + response
Model-->>StreamLoop: ResponseReasoningTextDeltaEvent / ResponseReasoningSummaryTextDeltaEvent
StreamLoop->>StreamLoop: append delta → _reasoning_snapshot
StreamLoop->>EventQueue: enqueue ReasoningDeltaEvent(delta, snapshot)
EventQueue-->>Client: stream_events() yields ReasoningDeltaEvent and other events
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/agents/run_internal/run_loop.py`:
- Around line 1113-1114: The _reasoning_snapshot field is never cleared across
stream retries, causing ReasoningDeltaEvent.snapshot to include duplicated text;
import ResponseCreatedEvent and, inside the event loop where stream events are
handled (the loop that processes events from stream_response_with_retry /
get_stream after rewind()), detect events of type ResponseCreatedEvent and reset
_reasoning_snapshot = "" when such an event is received to mark a fresh response
attempt; ensure you reference and update the existing _reasoning_snapshot
variable (not a new local) so subsequent ReasoningDeltaEvent handling produces a
clean snapshot for the new stream.
In `@tests/test_reasoning_delta_stream_event.py`:
- Around line 100-104: The test currently breaks out when a ReasoningDeltaEvent
is seen but does nothing if none are emitted; update the test to explicitly fail
when no ReasoningDeltaEvent is observed by either setting a flag (e.g.,
seen_reasoning = False) and asserting seen_reasoning is True after the async for
loop, or by using an else branch on the loop to raise an AssertionError;
reference the async iterator result.stream_events(), the ReasoningDeltaEvent
type check and the event.type assertion to locate where to add the post-loop
failure assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: da5ae3ef-04e2-4c5c-a0aa-b83bf9abdb96
📒 Files selected for processing (5)
src/agents/__init__.pysrc/agents/run_internal/run_loop.pysrc/agents/stream_events.pytests/test_reasoning_delta_stream_event.pytests/test_stream_events.py
- Import ResponseCreatedEvent and reset _reasoning_snapshot to "" when a ResponseCreatedEvent is received inside the retry stream loop, fixing the bug where snapshot text would be duplicated across retries - In test_reasoning_delta_event_type_field: add found=False flag and assert found after the loop so the test properly fails when no ReasoningDeltaEvent is emitted
- Import ResponseCreatedEvent and reset _reasoning_snapshot to "" when a ResponseCreatedEvent is received inside the retry stream loop, fixing the bug where snapshot text would be duplicated across retries - In test_reasoning_delta_event_type_field: add found=False flag and assert found after the loop so the test properly fails when no ReasoningDeltaEvent is emitted
01d8b3d to
996be13
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_reasoning_delta_stream_event.py`:
- Around line 59-70: The test currently allows a vacuous pass when no
ReasoningDeltaEvent snapshots are emitted; update the test around the snapshots
collection from result.stream_events() to require at least one snapshot before
performing length-order and content checks: after collecting snapshots (variable
snapshots) add an assertion that snapshots is not empty (e.g., assert snapshots,
"no reasoning snapshots emitted") so the subsequent loop and final check that
"Hello world" appears in snapshots[-1] will fail if no ReasoningDeltaEvent
objects were produced.
- Around line 82-85: The test currently only asserts that no individual event is
a ReasoningDeltaEvent but doesn't ensure the stream produced any events; update
the test that uses result.stream_events() to also verify the stream yielded at
least one event (e.g., accumulate events or increment a counter while iterating)
and assert the collected events list length (or counter) is greater than zero,
while still asserting none of the yielded events are instances of
ReasoningDeltaEvent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b02a6b46-9175-4392-97c4-428c546aef1f
📒 Files selected for processing (5)
src/agents/__init__.pysrc/agents/run_internal/run_loop.pysrc/agents/stream_events.pytests/test_reasoning_delta_stream_event.pytests/test_stream_events.py
✅ Files skipped from review due to trivial changes (1)
- src/agents/run_internal/run_loop.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/agents/init.py
Instead of trying to constraint the agent to only work in the workspace, allow it to fail freely without sudo.
…i#2850) Co-authored-by: Kazuhiro Sera <seratch@openai.com>
### Summary This pull request fixes sandbox compaction defaults so Azure/custom deployment names do not fail before the model request is sent. It makes compaction model-window lookup separator-insensitive for known OpenAI-style names like `gpt-5-2`, adds a non-throwing lookup path, and falls back to the static compaction threshold when the model window cannot be inferred. ### Test plan - `make format` - `make lint` - `uv run pytest tests/sandbox/test_compaction.py tests/sandbox/capabilities/test_compaction_capability.py tests/test_sandbox_runtime_agent_preparation.py -q` - `uv run mypy src/agents/sandbox/capabilities/compaction.py tests/sandbox/test_compaction.py tests/sandbox/capabilities/test_compaction_capability.py` - `uv run pyright src/agents/sandbox/capabilities/compaction.py tests/sandbox/test_compaction.py tests/sandbox/capabilities/test_compaction_capability.py` Full `make tests` / `make typecheck` are currently blocked in this workspace by missing optional dependencies and unrelated existing failures (`numpy`, `litellm`, `sqlalchemy`, `temporalio`, `boto3`, runloop extra, and one PTY timing assertion). ### Issue number Refs openai#2927 ### Checks - [x] I've added new tests (if relevant) - [ ] I've added/updated the relevant documentation - [x] I've run `make lint` and `make format` - [ ] I've made sure tests pass
- Add SandboxPathGrant manifest support for explicit access to absolute paths outside sandbox workspace. - Centralize path handling in WorkspacePathPolicy.normalize_path(...), including extra grant matching, symlink-aware host validation, and most-specific grant selection. - Harden access boundaries by rejecting filesystem-root grants, // root aliases, and grants that resolve to /. - Preserve nested grant semantics, including writable parent + read-only child cases through remote symlink targets and macOS exec confinement. - Update sandbox provider integrations to use shared path policy across Docker, Unix-local, Runloop, Vercel, Cloudflare, E2B, Modal, Daytona, and Blaxel.
Co-authored-by: Kazuhiro Sera <seratch@openai.com>
## Summary - Refresh runtime handling around session and tool-call flows. - Adjust model configuration metadata used by runtime integrations. - Add focused coverage for the updated behavior. ## Validation - .venv/bin/python -m pytest tests/model_settings/test_serialization.py tests/models/test_trace_config.py tests/mcp/test_streamable_http_client_factory.py tests/test_run_context_approvals.py tests/test_run_state.py::TestRunState::test_trace_api_key_serialization_is_opt_in tests/realtime/test_session.py - .venv/bin/ruff check <touched files> - .venv/bin/ruff format --check <touched files> - git diff --check
…tItem from agents (openai#3490)
…as (openai#825) Add a new ReasoningDeltaEvent to StreamEvent so callers can react to reasoning/thinking tokens in real time without unpacking low-level raw response events. The event is emitted whenever a ResponseReasoningSummaryTextDeltaEvent (o-series extended thinking via the Responses API) or a ResponseReasoningTextDeltaEvent (third-party models like DeepSeek-R1 via LiteLLM) passes through the stream. The underlying RawResponsesStreamEvent is still emitted as well, so nothing breaks for consumers that already inspect raw events. Fields: delta - the incremental text fragment from this chunk snapshot - full accumulated reasoning text so far in this turn type - always 'reasoning_delta' Closes openai#825
- Import ResponseCreatedEvent and reset _reasoning_snapshot to "" when a ResponseCreatedEvent is received inside the retry stream loop, fixing the bug where snapshot text would be duplicated across retries - In test_reasoning_delta_event_type_field: add found=False flag and assert found after the loop so the test properly fails when no ReasoningDeltaEvent is emitted
The two stream-event tests were only asserting on data conditional on a
ReasoningDeltaEvent being emitted at all, so a regression that stopped
emitting the event entirely would have passed silently.
* test_reasoning_delta_snapshot_accumulates: assert that snapshots is
non-empty before checking monotonic length and the "Hello world"
inclusion (previously gated on `if snapshots:`).
* test_no_reasoning_delta_event_without_reasoning: count yielded events
and assert the stream produced at least one, so the negative
not-isinstance assertion can't pass on an empty event stream.
Picked up the remaining nitpicks from the CodeRabbit review of PR #3.
5c36515 to
5b04cf8
Compare
…ning_deltas Default the new ReasoningDeltaEvent emission to off so the streamed event count is unchanged for existing consumers. Opt in via RunConfig.emit_reasoning_deltas=True to receive reasoning deltas without unwrapping the raw events. Addresses the event-volume concern raised in review.
Adding ReasoningDeltaEvent to the StreamEvent union widened PrintableEvent, so mypy could no longer prove the trailing match handled a RunItemStreamEvent. Add an early return for ReasoningDeltaEvent, mirroring the existing RawResponsesStreamEvent guard, which restores narrowing and fixes typecheck.
|
Closing this fork staging PR. The corresponding upstream work is tracked separately, so this internal branch is no longer needed. |
Summary
When models like o3 or DeepSeek-R1 produce reasoning/thinking tokens during streaming, those deltas currently only surface as raw
RawResponsesStreamEventwrappers around low-levelresponse.reasoning_summary_text.deltaorresponse.reasoning_text.deltaevents. To consume them, callers have to inspect.data.typeand cast the event themselves — there's no clean signal in theStreamEventunion.This PR adds
ReasoningDeltaEventtoStreamEventand emits it alongside the existing raw event so reasoning deltas are as easy to consume as message deltas.Closes openai#825
What changed
ReasoningDeltaEventdataclass tostream_events.pywithdelta,snapshot, andtypefieldsStreamEventtype alias to includeReasoningDeltaEventagents/__init__.pyrun_internal/run_loop.py, therun_single_turn_streamedloop now emits aReasoningDeltaEventafter eachResponseReasoningSummaryTextDeltaEvent(o-series) andResponseReasoningTextDeltaEvent(DeepSeek/LiteLLM)snapshotfield accumulates the full reasoning text so far in the turn, so callers don't have to maintain their own bufferUsage example
Tests
Added
tests/test_reasoning_delta_stream_event.pycovering:ReasoningDeltaEventis emitted for reasoning itemsagentsAlso updated
tests/test_stream_events.py::test_complete_streaming_eventsto account for the new event in the event sequence (count goes from 27 → 28).Summary by CodeRabbit
New Features
Tests