Skip to content

Conversation

@lochmueller
Copy link
Contributor

@lochmueller lochmueller commented Dec 18, 2025

Q A
Bug fix? yes
New feature? no
Docs? no
Issues
License MIT

Fix:

@carsonbot carsonbot added Status: Needs Review Bug Something isn't working Platform Issues & PRs about the AI Platform component labels Dec 18, 2025
@OskarStark OskarStark changed the title [Platform] Streamline RawHttpResult for streaming [Platform] Streamline RawHttpResult for streaming Dec 18, 2025
@OskarStark
Copy link
Contributor

Can you please rebase?

@lochmueller lochmueller requested a review from Nyholm as a code owner December 18, 2025 12:40
@lochmueller lochmueller force-pushed the feature/streamline-raw-http-result branch from a325bd0 to 9ac792b Compare December 18, 2025 12:41
Fix:
- Skip SSE comments
- Skip SSE errors
- Drop manual JSON convert process
- Use JSON decode from the ServerSentEvent
@lochmueller lochmueller force-pushed the feature/streamline-raw-http-result branch from 9ac792b to 1645be2 Compare December 18, 2025 12:48
@lochmueller
Copy link
Contributor Author

Can you please rebase?

done. The workflow errors are not related to my changes...

@OskarStark
Copy link
Contributor

Lets merge #1204 first, rebase and adjust the testcase

OskarStark added a commit that referenced this pull request Dec 18, 2025
This PR was merged into the main branch.

Discussion
----------

[Platform] Add test for `RawHttpResult`

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| Docs?         | no
| Issues        | --
| License       | MIT

Should be merged before #1191

Commits
-------

ff0ec93 [Platform] Add test for RawHttpResult
@OskarStark
Copy link
Contributor

I merged my PR, can you please update the test? thanks

Comment on lines -50 to -59
// Split in case of multiple JSON objects
$deltas = explode(",\r\n", $jsonDelta);

foreach ($deltas as $delta) {
if ('' === trim($delta)) {
continue;
}

yield json_decode($delta, true, flags: \JSON_THROW_ON_ERROR);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the reason for removing this?
there is no replacement for this part or do i miss something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

background for having this was gemini api streaming parts of a larger json document - which cannot be decoded right away - testing with examples/gemini/stream.php might help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working Platform Issues & PRs about the AI Platform component Status: Needs Work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants