fix(open_responses): tolerate streaming events missing index fields#243
Merged
Conversation
llama.cpp's Responses implementation omits the spec-required positional index fields (output_index, content_index, annotation_index, summary_index), so StreamingEvent.fromJson threw "Null is not a subtype of int" and the stream died. Default these fields to 0 in the fromJson factories, mirroring the existing sequence_number handling. Fields stay non-nullable (int), so this is non-breaking. Closes #242
There was a problem hiding this comment.
Pull request overview
Improves open_responses streaming robustness by making StreamingEvent.fromJson tolerate OpenAI-compatible servers (e.g., llama.cpp) that omit spec-required positional index fields, preventing runtime type-cast crashes during streaming.
Changes:
- Default missing
output_index,content_index,annotation_index, andsummary_indexfields to0in streaming eventfromJsonfactories (instead of hard-casting to non-nullableint). - Add/adjust inline documentation on affected index fields to clarify defaulting behavior for non-conformant providers.
- Add regression tests to ensure minimal/llama.cpp-shaped streaming JSON parses successfully and defaults indices/sequence numbers to
0.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/open_responses/lib/src/models/streaming/streaming_event.dart | Makes index parsing null-tolerant by defaulting missing positional indices to 0 to avoid stream crashes. |
| packages/open_responses/test/unit/models/streaming_event_test.dart | Adds regression coverage for minimal-server events missing index fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
llama.cpp's llama-server exposes the OpenAI-compatible Responses surface under /v1; with the missing-index-field fix in this PR the client now works against it. List it alongside the other local runtimes.
The fix also made annotation_index (output_text.annotation.added) and summary_index (reasoning_summary_part.added) default to 0, but those paths lacked direct regression coverage. Add tests parsing both events with all index fields omitted (PR review by @Copilot).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
llama.cpp(and other minimal OpenAI-compatible servers):StreamingEvent.fromJsonthrewtype 'Null' is not a subtype of type 'int', killing the stream.llama.cpp's Responses implementation omits the positional index fields the OpenAPI spec marks as required —output_index,content_index,annotation_index,summary_index— which our hand-written factories were hard-casting as non-nullableint.0when absent, so streaming works against servers that don't emit them while remaining unchanged for compliant servers (OpenAI, etc.).llama.cppto the Supported providers table in the package README.Details
The
fromJsonfactories instreaming_event.dartpreviously did:This now mirrors the handling already in place for
sequence_number(which is also spec-required and also omitted byllama.cpp):Why default-to-0 instead of making the fields nullable (
int?):event.outputIndexstaysint; no consumer null-checks required (nullable would be a major-version change).sequence_numberconvention, the most analogous "ordering metadata some providers omit" field.0.fromJsonfactories change;toJson/copyWith/==/hashCode/toStringare untouched because the field types are unchanged, so we still emit spec-conformant output.Applied across all 27 affected casts (15×
output_index, 9×content_index, 1×annotation_index, 2×summary_index). Verified safe for downstream consumers:streaming_event_accumulator,streaming_extensions, andstreaming_resourcecorrelate deltas byitem_id/ event type — never by these indices — so defaulting to0has no behavioral side effects.item_id(requiredString) is intentionally left as-is:llama.cppdoes emit it, and keeping it required preserves type safety where the data is present.This is a client-robustness fix; the OpenAPI spec is correct and unchanged.
References
/v1/responsessupport in llama.cpp server (the implementation users are hitting).Test Plan
open_responses(dart test test/unit/— 438 passed, 3 env-skipped)output_text.delta,content_part.added, andoutput_item.addedparsed fromllama.cpp-shaped JSON (nosequence_number/output_index/content_index) default to0instead of throwingdart formatclean;dart analyzeintroduces no new issuestoJsonround-trip still verified by existing testsCloses #242