Add stream contract details - #642
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a typed streaming event model to make the SSE streaming contract explicit and structurally verifiable end-to-end. It extends the core streaming surface (Runner.stream → Runtime.stream → StreamChunk → SSE frames) so clients can distinguish assistant text from boundaries/tool/reasoning events while preserving the existing delta-concatenation path for plain-text consumers.
Changes:
- Add
core/events.pydefining a discriminatedStreamEventunion and event classes (message boundaries, text deltas, tool calls, reasoning, steps). - Extend
StreamChunkwith aneventfield and updateRuntime.streamto yield typed events, including transitional normalization of legacystrstreams into a synthetic assistant message. - Update tests (core + example) and the spec plan to assert the new wire shape (
event+ optionaldelta, boundary frames, terminaldoneframe withoutevent/delta).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/api/pydanticai-streaming/app_test.py | Strengthens SSE contract assertions (typed event, boundary frames, and delta accumulation guarded by key presence). |
| docs/specs/523-ag-ui-support/plan.md | Updates verification guidance and inventories example/doc surfaces impacted by the new frame shape. |
| ak-py/tests/test_stream_events.py | Adds conformance tests for the StreamEvent discriminated union (round-trip, JSON-serializable, unique discriminators). |
| ak-py/tests/test_runtime_stream_events.py | Adds regression tests for Runtime.stream projection rules, hook behavior, and transitional str normalization. |
| ak-py/tests/test_pipeline_request_handler.py | Updates SSE wire-shape assertions to include event and boundary frames. |
| ak-py/src/agentkernel/framework/smolagents/smolagents.py | Declares non-streaming support via supports_streaming=False and updates the stream signature. |
| ak-py/src/agentkernel/framework/crewai/crewai.py | Declares non-streaming support via supports_streaming=False and updates the stream signature. |
| ak-py/src/agentkernel/core/runtime.py | Implements event-aware streaming, hook application to text-bearing events, and transitional str→event normalization. |
| ak-py/src/agentkernel/core/model.py | Adds `event: StreamEvent |
| ak-py/src/agentkernel/core/events.py | New typed event model and discriminated union for streaming. |
| ak-py/src/agentkernel/core/base.py | Updates the Runner streaming contract and adds supports_streaming. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
amithad
left a comment
There was a problem hiding this comment.
PR 1 of the #523 stack (the streaming contract): implementation matches spec.md §1-§4, the updated plan.md's factual claims verify against the base branch, and CI is fully green. No blockers; 2 suggestions and 2 nits inline, 1 question below.
Spec documents first (plan.md is the only spec doc in the diff; design.md/spec.md on the base branch were used as the requirements source):
- plan.md delta: claims verified. Every checkable claim added in this PR held up: the five example READMEs found by the shape grep, the three pre-existing docs showing a terminal frame carrying a
delta(rest-api.md:353,aws-serverless.md:911,streaming-openai/README.md:126), the 18versioned_docssnapshots, zero.deltamatches in example JS/HTML, onlypydanticai-streaminghaving a frame-assertingapp_test.py,.github/test-config.yaml:104, andapp_test.py:79's previously unguarded join. One internal inconsistency flagged inline (Iteration 1 Goal vs its updated Verify). - Spec conformance: complete for PR 1's slice. The event model,
StreamChunk.event, thesupports_streamingproperty (with CrewAI/smolagents declaringFalse), and theRuntime.streamrewrite implement all six §4 rules, matching the spec's code near-verbatim. The tests assert exactly the regression guards spec.md's Testing section demands (hook write-back, whole-chunk drop, non-text events skipping hooks, reasoning out ofdelta, the transitionalstrpath through the hook chain, no boundaries on a fully redacted stream), plus one beyond it (a migrated adapter gets no synthetic boundaries). The "exactly two edits to existing tests" gate holds in this diff. - [question] design.md:251 still claims a user-written frontend reading the SSE stream "is not broken", while this PR's plan.md update records that claim as "too strong" (an unguarded
frame.deltareader now appendsundefinedtwice per response). design.md is not in this diff so it cannot take an inline comment; consider carrying the same correction there so the two documents agree. - Process nits (not inline-anchorable): the PR description is the untouched template (no summary, no linked issue, empty checklists), and the title has no Conventional Commits type; something like
feat: typed stream events for the streaming contract (#523)would match repo convention. - Skipped as duplicate: the
events.pymodule naming (singular) is already raised on this PR.
432efa2 to
8b2e797
Compare
8b2e797 to
f1a3e2e
Compare
f1a3e2e to
9e136c8
Compare
Description
Type of Change
Related Issues
Fixes #
Relates to #
Changes Made
Testing
Checklist
Screenshots (if applicable)
Additional Notes