feat: Add rawResponse property to OrchestrationStreamResponse& getRequestId-helpers

[davidkna-sap]

Context

Closes SAP/ai-sdk-js-backlog#457.

What this PR does and why it is needed

[davidkna-sap]

I called it rawHttpResponse instead of rawResponse to stress that it doesn't contain the actual data being streamed.

[jerome-benoit]

It breaks namespace consistency across the API.
The stream use case is orthogonal, so the collision is not a concern here.

[davidkna-sap]

@jerome-benoit Please clarify what you mean with "namespace consistency across the API" in this context. Would you prefer it being called rawResponse? Given the other review comments I will be renaming it to rawResponse.

[jerome-benoit]

@jerome-benoit Please clarify what you mean with "namespace consistency across the API" in this context. Would you prefer it being called rawResponse? Given the other review comments I will be renaming it to rawResponse.

Two different transport layers - slightly -, same overall semantic expected on both content => the namespace should match.

[davidkna-sap]

The initial streaming HTTP response does not actually contain a X-Request-ID, so I also additionally also added getRequestId()-helpers for the orchestration client response objects, which just read this._data.request_id.

[davidkna-sap]

This file did not have tests yet, the AI decided to generate a bunch for things that were not covered.
I kept these to increase overall code coverage, but I would be glad to remove additional tests unrelated to the primary PR objective to ease reviewing.

[ZhongpinWang]

[pp] I prefer adding this test file in a separate PR. This file is a bit too much to be considered as a small improvement. But thanks for considering the test coverage and please do create a follow-up for this. 👍

[ZhongpinWang]

The idea if I understand correctly, is just to expose the HttpResponse as part of the OrchestrationStreamResponse object. HttpResponse contains headers, status, data, and every other things that is returned and set by Axios. In case of streaming, data property contains a Node.js Readable object, which users can parse by themselves. (Need to check though if the stream is for sure not consumed by us if user does not iterate the async iterable causing the Readable object been consumed and SSE been parsed.)

[ZhongpinWang]

This HttpResponse contains data property, which is a Node.js Readable stream object set by Axios.

See how we are parsing the SSE.

ai-sdk-js/packages/orchestration/src/orchestration-stream.ts

Line 23 in 4c5c6b6

SseStream.transformToSseStream<CompletionPostResponseStreaming>(

ai-sdk-js/packages/core/src/stream/sse-stream.ts

Line 101 in 4c5c6b6

const iter = response.data;

The idea is to expose this HttpResponse object in OrchestrationStreamResponse so that users can parse the data SSE by themselves. And if they do parse by themselves, then OrchestrationStreamResponse cannot be used but then it is user's choice.

But please double check, if data is consumable. My understanding is that if user consumes the stream by running a for await loop, then the stream is not consumable any more. So please check if it is not consumed by a for await, then can the Readable object still be consumed directly from the raw response.

[davidkna-sap]

I have confirmed data can be streamed, but decided against adding an e2e-test for this feature for now. Edit: I added a unit test for this.

[ZhongpinWang]

[pp/req] Unfortunately, we have exposed OrchestrationStreamResponse not just as a type. Adding an additional required property in the constructor is a breaking change. Please consider using setter later.

Or perhaps we accept this as a compatibility note. WDYT?

[davidkna-sap]

I did add a compatibility note.

I decided on doing it this way because it means that the new property does not have to be optional, and OrchestrationResponse also add the raw response via a constructor argument.
And while this is a breaking change at the type-level, it doesn't break anything at runtime.

[ZhongpinWang]

Make sense. Would of course be nicer if we have exported this as a type only

ai-sdk-js/packages/orchestration/src/index.ts

Line 46 in 4c5c6b6

export { OrchestrationStreamResponse } from './orchestration-stream-response.js';

Perhaps something you can consider doing, also for other response types.

[davidkna-sap]

I've changed the code to handle this gracefully by deprecating the constructor without the OrchestrationResponse, and throwing if OrchestrationResponse is undefined when rawResponse is accessed.

[ZhongpinWang]

[pp] I prefer adding this test file in a separate PR. This file is a bit too much to be considered as a small improvement. But thanks for considering the test coverage and please do create a follow-up for this. 👍

[ZhongpinWang]

Also please call this property rawResponse same as in the non-streaming response. (I see that we have some inconsistency between embedding and chat client... embedding response calls this response instead of rawResponse... But let's be at least consistent within one client.)

[ZhongpinWang]

[pp] If you want, consider deprecate response in embedding client and rename it to rawResponse and check docs. Cosmetic stuff cc @KavithaSiva WDYT?

[hyperspace-insights]

The pull request adds rawResponse property and getRequestId() helpers to orchestration response classes. I identified three related issues around type consistency and defensive programming for the getRequestId() implementations. The streaming version returns string | undefined while the non-streaming versions return string, but all read from the same field that could theoretically be missing until data is received. This inconsistency should be addressed to prevent potential runtime errors.

PR Bot Information

Version: 1.17.50 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback

• Correlation ID: fbca84c0-01c3-11f1-911c-0616ae270023
• Event Trigger: issue_comment.created
• LLM: anthropic--claude-4.5-sonnet

[ZhongpinWang]

lgtm