feat(middleware): result handling, model-state isolation, and system-prompt fidelity#2812
feat(middleware): result handling, model-state isolation, and system-prompt fidelity#2812zastrowm wants to merge 2 commits into
Conversation
…prompt fidelity Builds on the internal InvokeModelStage middleware (strands-agents#2760) with correctness fixes and an extensible result surface. Result handling: - Drop the MiddlewareResult sentinel from the event stream; the last yielded event is the result, matching the SDK's ModelStopReason / ToolResultEvent convention. - Output-phase handlers take and return a MiddlewareResult wrapper (with a .replace() helper) so we can extend the result later without changing the handler signature. - Type InvokeModelStage as MiddlewareStage[InvokeModelContext, ModelStopReason, TypedEvent]. Model-state isolation (matches TS): - Snapshot agent model_state before the chain, stream against the snapshot, and write back only on success. Middleware cannot leak state into the model call before or after next(). model_state is not exposed on the context. System-prompt fidelity: - Prefer system-prompt content blocks over the concatenated string so cachePoints survive the middleware round-trip; the terminal splits back into both forms for Model.stream(). Tests: - Python: model-state writeback/deletions/before-after-next, system-prompt string/ blocks/transform, CancelledError + multi-layer interrupt + inner-middleware-throw finally cases. Helper models moved to module top. - TS: Output tool-dispatch-prevention and message-append coverage.
| agent._model_state = model_state_snapshot | ||
|
|
||
| # The last event from the chain is ModelStopReason (the authoritative result) | ||
| stop_reason, message, usage, metrics = event["stop"] |
There was a problem hiding this comment.
Issue: event here is the async for loop variable, so if the middleware chain yields zero events it's never bound and this line raises UnboundLocalError: cannot access local variable 'event'. The previous code guarded this with an explicit if model_result is None: raise RuntimeError("Middleware chain did not produce a MiddlewareResult..."), which gave middleware authors a clear, actionable message. That guard is gone now.
This is reachable today — a Wrap middleware that short-circuits without yielding the result event triggers it. Reproduced:
async def silent(context, next_fn):
if False:
yield # never yields the result event
agent._middleware_registry.add_middleware(InvokeModelStage, silent)
agent("test") # -> UnboundLocalError at this lineSuggestion: Restore an explicit guard so a misbehaving middleware fails loudly instead of with a cryptic UnboundLocalError. For example, sentinel-initialize before the loop and check after:
last_event = None
async for event in agent._middleware_registry.invoke(...):
last_event = event
yield event
if last_event is None:
raise RuntimeError(
"Middleware chain did not yield a result event. "
"Ensure middleware forwards events from next()."
)
agent._model_state = model_state_snapshot
stop_reason, message, usage, metrics = last_event["stop"]| transformed = handler(MiddlewareResult(value=last_event)) | ||
| if inspect.isawaitable(transformed): | ||
| transformed = await transformed | ||
| yield transformed.value |
There was a problem hiding this comment.
Issue: If an Output handler forgets to return (returns None), this line raises AttributeError: 'NoneType' object has no attribute 'value', which doesn't point the author at the real mistake. The contract (handler must return a MiddlewareResult) is documented, but the failure mode is opaque.
Suggestion: Consider a targeted check after awaiting transformed to produce a clearer message, e.g.:
if not isinstance(transformed, MiddlewareResult):
raise TypeError(
f"Output handler must return a MiddlewareResult, got {type(transformed).__name__}"
)
yield transformed.valueOptional/low-priority since this is an internal stage, but it's a cheap guardrail given the wrapper contract is new for middleware authors.
|
Assessment: Comment Clean, well-tested port. The result-encoding simplification (last event is the result) and the model-state snapshot/writeback isolation are both clear improvements over the sentinel approach, and the system-prompt content-block fidelity fix is a nice correctness catch. 97 unit tests pass locally and they assert on observable behavior (full messages, model-received state) rather than internal plumbing. Review themes
Nice work keeping the chain transparent while still giving Output handlers room to evolve via the |
Add explicit guards so middleware authors get clear messages: - RuntimeError if a Wrap handler yields zero events (was UnboundLocalError) - TypeError if an Output handler forgets to return MiddlewareResult (was AttributeError)
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Description
Follow-up to the internal
InvokeModelStagemiddleware (#2760), aligning the Python port with the TypeScript behavioral spec. Two behavior changes.1. Result handling. The
MiddlewareResultsentinel no longer rides the event stream. Instead the last yielded event is the result, matching the SDK convention (ModelStopReasonendsstream_messages(),ToolResultEventends tool execution). The chain is now transparent. Output handlers — which transform the result — take/return aMiddlewareResultwrapper so we can extend it later without changing the signature.2. Model-state isolation. Model runtime state is snapshotted before the chain and written back only on success, so middleware can't corrupt provider state (e.g. server-side response IDs) via the
agentescape hatch.Public API Changes
InvokeModelStageis internal, so no public surface changes. For middleware authors, the Output-phase handler shape changes to aMiddlewareResultwrapper:Wrap and Input handlers are unchanged.
Related Issues
strands-ts/src/middleware/README.mdDocumentation PR
None — internal stage; divergences documented in
strands-py/src/strands/_middleware/README.md.Type of Change
New feature
Testing
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.