Propagate ContinuationToken through Response.ToUpdates so it survives a ToUpdates/Collect round-trip - #705
Conversation
There was a problem hiding this comment.
Pull request overview
Propagates ContinuationToken through Response.ToUpdates() so a Response can round-trip through ToUpdates + Update/Collect without losing the continuation token, aligning behavior with other SDKs.
Changes:
- Emit an extra metadata
ResponseUpdatewhenResponse.ContinuationTokenis non-empty, and copy the token onto that update. - Add a regression test ensuring
ContinuationTokensurvives aToUpdates→Update/Coalesceround-trip.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| agent/response.go | Include ContinuationToken in the metadata-only “extra update” path so it can be reconstructed by collection. |
| agent/response_test.go | Add a round-trip test to verify ContinuationToken is preserved. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment has been minimized.
This comment has been minimized.
a22e442 to
cbc8057
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ToUpdates copied response-level FinishReason, CreatedAt, AdditionalProperties and Usage onto emitted updates but omitted ContinuationToken. Since Collect clears the token on any empty-token update, a Response carrying a token lost it after a ToUpdates/Collect round-trip. Set it on the trailing update, matching .NET AsChatResponseUpdate.
…s metadata update
9ca0b4c to
514d8cb
Compare
Parity Review — ✅ ApprovedScope: What changed
Cross-repo alignmentThe PR description correctly identifies the upstream .NET reference. In No equivalent Python surface for API surfaceNo exported identifiers were added, removed, or renamed. The VerdictThe change is a targeted bug fix that moves Go closer to .NET parity. No cross-SDK consistency issues found. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
What
Response.ToUpdates()copies response-levelFinishReason,CreatedAt,AdditionalPropertiesandUsageonto the emittedResponseUpdatevalues, but never setContinuationToken, even though bothResponseandResponseUpdatecarry that field.Because
Response.Update()resetsresp.ContinuationTokento empty for every update whose token is empty, and everyToUpdates-emitted update had an empty token, aResponse{ContinuationToken: tok}converted viaToUpdatesand re-collected yielded an emptyContinuationToken.This extends the trailing extra-update condition to also fire when
ContinuationTokenis non-empty, and setsextra.ContinuationTokenon it so a subsequentCollectreconstructs the token.Why
Four sibling response-level fields were already copied in the same function while
ContinuationTokenalone was omitted. This aligns with .NET, whereAsChatResponseUpdatecopiesContinuationToken, keeping the ToUpdates/Collect round-trip lossless across SDKs.Testing
Added
TestResponse_ToUpdates_PropagatesContinuationTokeninagent/response_test.go: it builds aResponsewith an assistant message andContinuationToken: tok-123, round-trips it throughToUpdatesthenUpdate/Coalesce, and asserts the token is preserved. The test fails before the change (empty token) and passes after.go build ./...,go vet ./agent/...andgo test ./agent/...are green.