Replay citation annotations on the Responses input path - #1101
PratikDhanave (PratikDhanave) wants to merge 1 commit into
Conversation
Building a Responses assistant turn from client-side history always sent an empty output_text annotations array, dropping any citations the text arrived with (a standing TODO). populateAnnotations maps inbound Responses citations onto CitationAnnotation, but the reverse was never implemented, so a prior assistant turn round-tripped through history forwarding with its url/file/ container citations stripped. Add annotationsToOutputText, the inverse of populateAnnotations, inferring each output-text annotation variant from the populated citation fields (as the Python client does). Regions without both span bounds are skipped, and a citation with multiple regions fans out to one annotation per region.
github-actions
Bot
added
area:provider
There was a problem hiding this comment.
🔵 Needs a closer look
Citation preservation issues remain, and several conversion paths and span edge cases are untested.
Pull request overview
Adds replay of Responses citation annotations when rebuilding assistant history.
Changes:
- Converts framework citations back to Responses annotations.
- Handles citation spans and multiple regions.
- Adds URL and file citation replay tests.
File summaries
| File | Summary |
|---|---|
provider/openaiprovider/responses.go |
Implements citation conversion and span handling; unresolved issues remain around citation variants, file indexes, and edge-case coverage. |
provider/openaiprovider/responses_test.go |
Tests URL and file replay, but lacks coverage for container, file-path, multi-region, and incomplete-span cases. |
Review details
Suppressed comments (4)
provider/openaiprovider/responses.go:1922
- The added test exercises only the URL and filename-bearing file-citation branches. The new container/file-path conversions and the
citationSpansedge cases (multiple regions and a missing bound) are untested, so regressions in three of the four variants or the span fan-out behavior would pass CI; add focused cases for those branches.
case containerID != "" && cit.FileID != "":
for _, span := range citationSpans(cit.AnnotatedRegions) {
out = append(out, responses.ResponseOutputTextAnnotationUnionParam{
OfContainerFileCitation: &responses.ResponseOutputTextAnnotationContainerFileCitationParam{
ContainerID: containerID,
provider/openaiprovider/responses.go:1946
- This replay drops the
indexcarried by Responsesfile_citationannotations. The Responses payloads in this package include that field (for example, the non-streaming fixture), andpopulateAnnotationscurrently retains the original variant inRawRepresentation; reconstructing every file citation without its original index changes non-zero citations (and can produce an invalid input annotation). Preserve the index through the framework representation or recover it from the raw annotation before building this param.
OfFileCitation: &responses.ResponseOutputTextAnnotationFileCitationParam{
FileID: cit.FileID,
Filename: cit.Title,
},
provider/openaiprovider/responses.go:1952
- This heuristic changes a valid
file_citationwith an empty filename into afile_pathannotation:populateAnnotationsalready accepts that shape (the existing streaming fixture atresponses_test.go:4454has one), andRawRepresentationis excluded from serialized client history, so the original variant cannot be recovered here. Preserve an explicit annotation variant through history (or otherwise retain the distinction) before choosing betweenfile_citationandfile_path; otherwise replay changes the citation type.
case cit.FileID != "":
out = append(out, responses.ResponseOutputTextAnnotationUnionParam{
OfFilePath: &responses.ResponseOutputTextAnnotationFilePathParam{
FileID: cit.FileID,
},
provider/openaiprovider/responses_test.go:1053
- This test covers only a URL citation with one complete span and a file citation. The new container/file-path branches and
citationSpansfan-out/partial-bound behavior remain untested, so regressions in the other variants can pass unnoticed; add cases for those paths (including multiple and incomplete regions).
&message.CitationAnnotation{
Title: "Example",
URL: "https://example.com",
AnnotatedRegions: message.AnnotatedRegions{&message.TextSpanAnnotatedRegion{StartIndex: &start, EndIndex: &end}},
},
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
github-actions
Bot
added
kind:code
|
Scope: user-visible behavior (internal helpers, but change is observable in the wire request Go sends to the OpenAI Responses API) Changed Go contract: no new exported API. Upstream evidence reviewed:
Result: findings reported. The Go port correctly mirrors Python's overall structure (container_file_citation / url_citation / file_citation / file_path branching, one annotation per annotated region) and this closes a real gap versus a standing
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent · copilot · auto · 88.6 AIC · ⌖ 6.75 AIC · ⊞ 9.2K
| // the citations it originally arrived with. Which variant to emit is inferred | ||
| // from the populated fields, mirroring how populateAnnotations maps each one. | ||
| // Annotations that are not citations are skipped. | ||
| func annotationsToOutputText(anns []message.Annotation) []responses.ResponseOutputTextAnnotationUnionParam { |
There was a problem hiding this comment.
Comparing this to the Python parity implementation _annotations_to_output_text (python/packages/openai/agent_framework_openai/_chat_client.py:460-529), the file_citation and file_path Responses branches there restore the original index value from additional_properties["index"] (populated on ingestion at _chat_client.py:3047-3062, and again in the streaming path at _chat_client.py:3744-3770, both storing "index": annotation.index).
ResponseOutputTextAnnotationFileCitationParam.Index and ResponseOutputTextAnnotationFilePathParam.Index are api:"required" fields on the OpenAI Go SDK (github.com/openai/openai-go/v3@v3.61.0 responses/response.go:21196-21208 and :21279-21289). annotationsToOutputText's cit.FileID != "" && cit.Title != "" and cit.FileID != "" branches never set Index, so it always serializes as 0.
This is tightly coupled to this PR: before this change annotations were always empty, so no incorrect index was ever sent; now that file_citation/file_path annotations are replayed, any assistant turn with more than one file citation (or a non-zero original index) will silently emit the wrong index on replay, diverging from the round-trip fidelity Python achieves via additional_properties["index"].
Root cause is that Go's message.CitationAnnotation (message/annotation.go:80-90) has no field to carry this "index" value from populateAnnotations (which also drops it today), so it cannot be reconstructed here. Suggest adding an index carrier (e.g. via AdditionalProperties["Index"], consistent with how ContainerId is already threaded through AdditionalProperties) in both populateAnnotations and annotationsToOutputText so this new replay path doesn't regress round-trip fidelity for multi-citation messages.
When a Responses assistant turn is rebuilt from client-side history,
responsesBuildMessageParamalways emitted an emptyoutput_textannotations array — a standing// TODO: Convert message annotations back to Responses output-text annotations.populateAnnotationsmaps inbound Responses citations (url_citation,file_citation,container_file_citation,file_path) ontomessage.CitationAnnotation, but the reverse was never implemented. So a prior assistant turn carrying url/file/container citations round-tripped through history forwarding with those citations stripped. The Python client recovers them via_annotations_to_output_textfor exactly this reason ("so assistant messages roundtrip cleanly through history forwarding").Change
annotationsToOutputText(the inverse ofpopulateAnnotations) and use it at the replay site instead of the empty slice. The output-text variant is inferred from the populated citation fields, mirroring howpopulateAnnotationsmaps each one.citationSpansto extract[start, end)pairs from annotated regions; regions missing either bound are skipped, and a citation with multiple regions fans out into one annotation per region (each Responses annotation carries a single span).No new public API — this uses the existing
message.CitationAnnotationtype.Test
TestResponsesAssistantReplayRoundTripsCitationAnnotations: replays an assistantTextContentcarrying a URL citation (with a span) and a file citation, and asserts the captured requestinput[].content[].annotationsreconstructs both. Fails before the change (empty annotations), passes after.