Forward provider updates in the structured-output middleware so the response is not empty - #607
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the structured-output provider middleware so it forwards streamed ResponseUpdates downstream while still accumulating text for final structured unmarshalling, preventing collected responses and history persistence from ending up empty on the happy path.
Changes:
- Forward provider updates from the structured-output middleware (and respect downstream stop/backpressure) while continuing to accumulate JSON text for unmarshalling.
- Extend the existing structured-output success test to assert the collected
Response.String()contains the streamed JSON payload. - Add a history-provider test ensuring assistant messages are actually persisted (non-empty) when structured output is enabled.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| agent/structuredoutput.go | Forwards updates in the structured-output middleware so downstream consumers (including history accumulation) receive assistant output. |
| agent/structuredoutput_test.go | Adds/extends tests to validate returned response text and persisted assistant messages are non-empty and match the streamed JSON payload. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The structured-output middleware accumulated provider updates to build the JSON payload but never yielded any update downstream, so agent.invoke saw zero updates and left historyResponse empty. As a result the collected Response had no text and no assistant message was persisted by the history provider, even though unmarshalling succeeded. Forward each non-nil update after accumulating it, stopping if the consumer stops. This surfaces the assistant response alongside the deserialized value, matching .NET AgentRunResponse<T> which keeps Messages/Text next to the parsed Result.
6bfdb0e to
4a5106e
Compare
Parity Review — PR #607Result: ✅ No parity issues found. This PR fixes a bug in Change SummaryThe three-line addition inserts if !yield(update, nil) {
return
}This ensures that:
Upstream AlignmentThe change moves Go toward parity with the .NET No new exported APIs were added; no defaults or option shapes changed. The modification is a narrow internal correctness fix.
|
What
The structured-output middleware's happy-path loop accumulates provider updates to assemble the JSON payload for unmarshalling, but it never yields any update downstream. The only success exit yields nothing.
Because this is a provider middleware and
agent.invokebuilds the response by consuming the provider-middleware chain (historyResponse.Update(update)per yielded update), emitting zero updates leaveshistoryResponseempty. The consequence: the collectedResponsehas empty text, and the history provider persists an empty assistant message even though the structured value was deserialized correctly.Fix
After appending the update text to the accumulation buffer and updating the message key, forward the update downstream and stop if the consumer stops:
This preserves the existing per-message reset logic and the final unmarshal, while surfacing the assistant response so
agent.invokepopulates and persistshistoryResponse.Parity
This matches .NET
AgentRunResponse<T>, which keepsMessages/Textalongside the deserializedResultrather than discarding the raw assistant response once the value is parsed.Tests
TestAgent_StructuredOutput_SuccessfulUnmarshalto assert the returnedResponse.String()equals the streamed JSON payload (previously discarded).TestAgent_StructuredOutput_StoresAssistantMessages, which runs with an explicit history provider and a session and asserts the stored assistant messages are non-empty and contain the JSON payload.Both tests fail before the change and pass after.
go build ./...,go vet ./agent/..., andgo test ./agent/...are green.