fix(google-generativeai): emit choice events from the last chunk - #4418
fix(google-generativeai): emit choice events from the last chunk#4418chiruu12 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughSynchronous and asynchronous Google Generative AI streaming builders now emit choice events from the final response chunk and skip emission when a stream is empty. ChangesGoogle Generative AI streaming events
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to The change is localized to streamed event emission and avoids the reported failure on empty streams; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-google-generativeai/tests/test_streaming_events.py (1)
93-101: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd an empty asynchronous stream test.
The current test covers only the synchronous empty-stream branch. Add the equivalent asynchronous case to protect
_abuild_from_streaming_response.Proposed async empty-stream regression test
+@pytest.mark.asyncio +async def test_async_streaming_with_no_chunks_does_not_emit(monkeypatch): + seen = _streaming_setup(monkeypatch) + + async def agen(): + for chunk in []: + yield chunk + + generator = _abuild_from_streaming_response( + MagicMock(), agen(), "gemini-2.0-flash", MagicMock(), None + ) + async for _ in generator: + pass + + assert "response" not in seen🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opentelemetry-instrumentation-google-generativeai/tests/test_streaming_events.py` around lines 93 - 101, Add an asynchronous counterpart to test_sync_streaming_with_no_chunks_does_not_emit that exercises _abuild_from_streaming_response with an empty async iterator, consumes the result, and asserts "response" is absent from seen while reusing the existing _streaming_setup setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/opentelemetry-instrumentation-google-generativeai/tests/test_streaming_events.py`:
- Around line 93-101: Add an asynchronous counterpart to
test_sync_streaming_with_no_chunks_does_not_emit that exercises
_abuild_from_streaming_response with an empty async iterator, consumes the
result, and asserts "response" is absent from seen while reusing the existing
_streaming_setup setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3776eafc-1614-4d1c-a2bb-390b95142320
📒 Files selected for processing (2)
packages/opentelemetry-instrumentation-google-generativeai/opentelemetry/instrumentation/google_generativeai/__init__.pypackages/opentelemetry-instrumentation-google-generativeai/tests/test_streaming_events.py
Both streaming builders passed the generator to
emit_choice_eventsafter thefor item in responseloop had already drained it.emit_choice_eventsreadsresponse.candidates, which a generator does not have, so with events enabled every streamed Gemini call raisedAttributeErrorout of the generator into the caller, and the span was never ended.The non-events branch three lines below already used
last_chunk. This makes the events branch do the same, and skips emitting when the stream produced no chunks at all.Neither builder is decorated
@dont_throwand neither ends the span in afinally, which is why this surfaced in user code rather than a log line. Worth fixing, but out of scope here.Tests in
tests/test_streaming_events.pycover the sync builder, the async builder and the empty stream. All three fail on main.Fixes #4417
Summary by CodeRabbit
Bug Fixes
Tests