Skip to content

refactor: enhance stream processing in Adapter class#28

Merged
eldadfux merged 16 commits intomainfrom
feat-better-sse
Mar 28, 2026
Merged

refactor: enhance stream processing in Adapter class#28
eldadfux merged 16 commits intomainfrom
feat-better-sse

Conversation

@eldadfux
Copy link
Copy Markdown
Member

  • Introduced methods for managing streaming buffers and processing lines from chunks.
  • Added constants and properties to handle incomplete SSE fragments.
  • Updated adapter implementations (Anthropic, Deepseek, Gemini, OpenAI, Perplexity, XAI) to utilize new stream processing methods, improving code clarity and maintainability.

- Introduced methods for managing streaming buffers and processing lines from chunks.
- Added constants and properties to handle incomplete SSE fragments.
- Updated adapter implementations (Anthropic, Deepseek, Gemini, OpenAI, Perplexity, XAI) to utilize new stream processing methods, improving code clarity and maintainability.
@eldadfux eldadfux marked this pull request as ready for review March 26, 2026 21:05
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 26, 2026

Greptile Summary

This PR centralises SSE stream processing in the base Adapter class, adding buffer management for incomplete SSE fragments that span HTTP chunk boundaries, shared helpers for line decoding and token emission, and try/finally guards to ensure the buffer is always cleaned up. All six provider adapters are migrated to the new helpers, and a comprehensive new test base (tests/Agents/Adapters/Adapter.php) covers adapter identity, model lists, token counters, timeouts, and embedding contracts.\n\nKey changes:\n- prepareStreamLines() carries incomplete SSE line fragments across chunks via $streamBuffer, fixing a pre-existing correctness gap for split chunks.\n- decodeSseJsonLine() / decodeJsonOrSseLine() / appendStreamToken() eliminate duplicated SSE parsing boilerplate across all adapters.\n- normalizeModelForCompatibilityChecks() in OpenAI provides a clean hook for subclasses (e.g. OpenRouter) to remap model IDs without duplicating the compatibility logic.\n- decodeJsonObject() in the base class has two minor issues: array_is_list([]) is true in PHP 8.1+, so empty JSON objects ({}) are incorrectly discarded; and the foreach/key-type loop immediately after is unreachable dead code since json_decode always produces string keys for JSON objects.\n- $data returned from prepareStreamLines() still includes the partial trailing fragment that was popped from $lines, creating a subtle semantic inconsistency that could mislead future callers using $data for error detection.

Confidence Score: 4/5

Safe to merge after addressing the empty-object edge case in decodeJsonObject; all other findings are non-blocking style/doc issues.

The core refactor is sound and well-tested. The array_is_list([]) === true bug in decodeJsonObject is real but benign in current usage (no provider emits a bare {} event). The remaining comments are style/clarity issues. No regression risk to primary streaming paths.

src/Agents/Adapter.php — specifically the decodeJsonObject method and the prepareStreamLines return-value semantics.

Important Files Changed

Filename Overview
src/Agents/Adapter.php Core refactor target — adds stream buffer management and SSE helpers; decodeJsonObject has a latent bug rejecting empty JSON objects and contains unreachable dead code in the key-type loop.
src/Agents/Adapters/Anthropic.php Migrated to shared stream helpers; correctly uses decodeJsonOrSseLine (matching the original "data: or raw JSON" logic) and wraps fetch in try/finally for buffer cleanup.
src/Agents/Adapters/OpenAI.php Migrated to shared stream helpers; adds normalizeModelForCompatibilityChecks() hook for subclasses — clean extension point for OpenRouter-style model routing.
src/Agents/Adapters/Deepseek.php Migrated cleanly to shared helpers; streaming is now guarded by try/finally and SSE parsing is centralised.
src/Agents/Adapters/Gemini.php Migrated cleanly; decodeSseJsonLine is appropriate for Gemini's alt=sse format.
src/Agents/Adapters/Perplexity.php Migrated to shared helpers; no behavioural changes beyond the refactor.
src/Agents/Adapters/XAI.php Migrated to shared helpers; no issues found.
tests/Agents/Adapters/Adapter.php New abstract test base covering identity, models, token counters, timeouts, embeddings, and endpoint round-trips — good coverage of adapter contract.
tests/Agents/Conversation/ConversationBase.php Strengthened testListener to verify streamed tokens equal the final response content — a meaningful improvement over the previous trivial "was called" check.

Reviews (1): Last reviewed commit: "fixes" | Re-trigger Greptile

eldadfux and others added 10 commits March 26, 2026 22:12
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
- Updated the Adapter class to enhance stream state management with a dedicated reset method.
- Refactored JSON decoding in Deepseek, Gemini, OpenAI, Perplexity, and XAI adapters to prioritize chunk data, improving error handling and data integrity.
- Adjusted stream line preparation to ensure only complete lines are returned, enhancing overall processing reliability.
- Introduced a new method `consumeStreamBufferLine` in the Adapter class to handle remaining buffered stream fragments.
- Added `flushBufferedStreamData` method in each adapter to process any buffered data after chunk processing.
- Refactored the `process` method in each adapter to utilize the new stream processing methods, improving data handling and integrity.
…sses

- Replaced the Text and Image message classes with a unified Message class to streamline message handling.
- Updated all relevant code and tests to utilize the new Message class, ensuring compatibility and functionality.
- Enhanced the Message class to include MIME type detection for better attachment management.
@eldadfux eldadfux merged commit e37acda into main Mar 28, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant