Skip to content

Streaming review - #1864

Open
arnabnandy7 wants to merge 2 commits into
embabel:mainfrom
arnabnandy7:fix/pr-1819-streaming-review
Open

Streaming review#1864
arnabnandy7 wants to merge 2 commits into
embabel:mainfrom
arnabnandy7:fix/pr-1819-streaming-review

Conversation

@arnabnandy7

Copy link
Copy Markdown
Contributor

Part of review fixes for PR #1819

@igordayen igordayen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@arnabnandy7 - thanks for addressing generate stream with thinking.
few comments to consider.
KDOC with examples could be helpful.
Performance test would be awesome, what is the avg overhead.
Thanks

@arnabnandy7
arnabnandy7 force-pushed the fix/pr-1819-streaming-review branch from 0f53a88 to 8dcca17 Compare August 2, 2026 16:20
@arnabnandy7

Copy link
Copy Markdown
Contributor Author

@igordayen I've implemented all requested review changes:

  • Expanded KDoc with chunked input → event output examples.
  • Documented why extractAllThinkingBlocks() cannot be reused without whole-response buffering.
  • Moved the parser into TaggedThinkingParser.kt.
  • Added a deterministic 50,000-character scalability regression.
  • Added warmed performance reporting without flaky timing assertions.

Verification:

Tests run: 49
Failures: 0
Errors: 0
BUILD SUCCESS

Performance across two warmed 30-run samples:

Input:             50,000 characters
Chunks:            1,001 × ~50 characters
Pass-through:      0.054–0.064 ms
Tagged parsing:    2.494–3.530 ms
Average overhead:  ~2.95 ms per stream
Per-chunk overhead: ~2.95 μs
Throughput:        ~16.6 million characters/second

@igordayen igordayen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@arnabnandy7 - requesting changes reletated to documenting algorithm and reusability. the code is SO critical that readability should be addressed 100%.
Thanks for understanding.
Regards, Igor

@arnabnandy7
arnabnandy7 force-pushed the fix/pr-1819-streaming-review branch from 8dcca17 to 79fec8f Compare August 3, 2026 17:44
@arnabnandy7
arnabnandy7 requested a review from igordayen August 3, 2026 17:48
@igordayen

Copy link
Copy Markdown
Contributor

Will be working on a simple yet practical solution post 1.5.0 with full code reusability from the blocking thinking path.

Also, in the output, think is not triggered by native thinking.

Appreciate if you please keep dev on hold, thanks.

@arnabnandy7
arnabnandy7 marked this pull request as draft August 4, 2026 15:28
jstar0 and others added 2 commits August 5, 2026 22:07
Signed-off-by: King Star <mcxin.y@gmail.com>
Signed-off-by: Arnab Nandy <arnab_nandy7@yahoo.com>
@arnabnandy7
arnabnandy7 force-pushed the fix/pr-1819-streaming-review branch from 79fec8f to 0c93d19 Compare August 5, 2026 17:08
@arnabnandy7
arnabnandy7 marked this pull request as ready for review August 5, 2026 17:10
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.

3 participants