Adds API for streaming and iterating at the same time #532

elijahbenizzy · 2025-04-19T21:08:31Z

This allows us to iterate over streaming outputs. E.G. for deep-research mode.

See #360

Changes

Adds stream_iterate, astream_iterate
Adds tests

How I tested this

Unit tests

Notes

Checklist

PR has an informative and human-readable title (this will be pulled into the release notes)
Changes are limited to a single goal (no scope creep)
Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
Any change in functionality is tested
New functions are documented (with a description, list of inputs, and expected output)
Placeholder code is flagged / future TODOs are captured in comments
Project documentation has been updated if adding/changing functionality.

Important

Adds stream_iterate and astream_iterate methods for synchronous and asynchronous iteration over streaming outputs in Application.

New Functionality:
- Adds stream_iterate and astream_iterate methods to Application in burr/core/application.py for synchronous and asynchronous iteration over streaming outputs.
Testing:
- Adds tests for stream_iterate and astream_iterate in tests/core/test_application.py, covering cases of exhausting and not exhausting intermediate generators.
Enums:
- Updates ExecuteMethod in burr/lifecycle/base.py to include stream_iterate and astream_iterate.

^{This description was created by}^{for ff9564b. You can customize this summary. It will automatically update as commits are pushed.}

github-actions · 2025-04-19T21:09:54Z

A preview of ff9564b is uploaded and can be seen here:

✨ https://burr.dagworks.io/pull/532 ✨

Changes may take a few minutes to propagate. Since this is a preview of production, content with draft: true will not be rendered. The source is here: https://github.com/DAGWorks-Inc/burr/tree/gh-pages/pull/532/

burr/core/application.py

This allows us to iterate over streaming outputs. E.G. for deep-research mode. See #360

elijahbenizzy · 2025-04-21T03:49:56Z

RC version: pip install burr==0.40.0rc1

Docs:

ellipsis-dev

Caution

Changes requested ❌

Reviewed everything up to ff9564b in 3 minutes and 39 seconds. Click for details.

Reviewed 232 lines of code in 3 files
Skipped 0 files when reviewing.
Skipped posting 14 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. tests/core/test_application.py:1888

Draft comment:
Redundant assertion: the length of post_stream_item_calls is checked twice for equality with 20. Consider removing one instance.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. tests/core/test_application.py:2044

Draft comment:
Also duplicated assertion for post_stream_item_calls length (20) appears in test_iterate. Consider consolidating the duplicate check.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. tests/core/test_application.py:3560

Draft comment:
Test for _remap_dunder_parameters is clear; ensure that for actions that expect mangling the implementation of __context is kept consistent. No immediate change needed, but maintain documentation on expected dunder mapping behavior.
Reason this comment was not posted:
Confidence changes required: 20% <= threshold 50% None

4. tests/core/test_application.py:3330

Draft comment:
Tests of lifecycle hook execution and context passing are thorough. Consider adding inline comments in recursive action tests to clarify expected behavior.
Reason this comment was not posted:
Confidence changes required: 20% <= threshold 50% None

5. tests/core/test_application.py:3560

Draft comment:
Tests for _remap_dunder_parameters using mangled names (e.g. ActionWithContextTracer) are very thorough. Consider adding a brief comment to clarify expected behavior for future readers.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. tests/core/test_application.py:3320

Draft comment:
In test_application_does_not_allow_dunderscore_inputs, the error message is matched with 'double underscore'. Confirm that the regex precisely captures the intended error and consider clarifying the rationale in a comment.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. tests/core/test_application.py:3385

Draft comment:
The recursive action test uses a comment 'Fork bomb!'. Although intentional, consider clarifying the base case and expected total count (63) in more detail for maintainability.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. tests/core/test_application.py:3646

Draft comment:
In test__process_control_flow_params, sorting of the halt_after list is used to assert expected action names. Ensure that ordering is intentional; if order doesn’t matter, consider adding a clarifying comment.
Reason this comment was not posted:
Confidence changes required: 50% <= threshold 50% None

9. burr/lifecycle/base.py:15

Draft comment:
Abstract hook classes are defined clearly. Consider adding docstrings or inline comments on the expected type of 'future_kwargs' to improve clarity for future maintainers.
Reason this comment was not posted:
Confidence changes required: 50% <= threshold 50% None

10. tests/core/test_application.py:3605

Draft comment:
Overall, this test file is very comprehensive. Consider splitting the tests into smaller modules (e.g. sync vs async, remap functions, persister errors) to improve maintainability.
Reason this comment was not posted:
Confidence changes required: 50% <= threshold 50% None

11. tests/core/test_application.py:460

Draft comment:
Typo: Remove the stray 'ß' in the comment that explains how the reducer deletes items.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

12. tests/core/test_application.py:1576

Draft comment:
Typo: 'quicly' should be corrected to 'quickly' in the comment describing the test that uses inputs to speed up the stream.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

13. tests/core/test_application.py:954

Draft comment:
Typo: Correct 'floating poit comparison problems' to 'floating point comparison problems'.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

14. tests/core/test_application.py:2368

Draft comment:
Typo: Replace 'steram' with 'stream' in the relevant comments.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_fKF73bXFd6WSw46f

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

ellipsis-dev · 2025-04-27T17:16:11Z

tests/core/test_application.py

+    assert len(stream_event_tracker.pre_start_stream_calls) == 2
+    assert len(stream_event_tracker.post_end_stream_calls) == 2
+    assert len(stream_event_tracker.post_stream_item_calls) == 20
+    assert len(stream_event_tracker.post_stream_item_calls) == 20


Duplicate assertion for stream_event_tracker.post_stream_item_calls appears twice (asserting length == 20). Consider removing the redundant check to avoid duplication.

Suggested change

assert len(stream_event_tracker.post_stream_item_calls) == 20

ellipsis-dev · 2025-04-27T17:16:11Z

burr/core/application.py

+        Note that there are control-flow complexities involved here -- we need to ensure that the iterator
+        is properly pushed along and the prior streaming results containers are all finished before going to the next one.
+
+        :param halt_after: Action names/tags to halt before


Documentation typo: In the docstrings for both stream_iterate and astream_iterate, the descriptions of the parameters halt_after and halt_before are either swapped or incomplete (_description_). Please update them to clearly differentiate: halt_after should indicate which actions/tags cause the stream to halt after the action completes, and halt_before should indicate the actions/tags that, if encountered, halt the iteration before the action runs.

Suggested change

:param halt_after: Action names/tags to halt before

:param halt_after: Action names/tags to halt after the action completes

skrawcz

Need to fix up docs and test nits.

skrawcz · 2025-04-27T23:57:47Z

burr/core/application.py

@@ -1604,7 +1604,7 @@ async def astream_result(
        which will be empty. Thus ``halt_after`` takes precedence -- if it is met, the streaming result container will contain the result of the
        halt_after condition.

-        The :py:class:`AsyncStreamingResultContainer <burr.core.action.AsyncStreamingResultContainer>` is meant as a convenience -- specifically this allows for


but the type annotation is async? is this supposed to be changed?

skrawcz · 2025-04-27T23:58:32Z

burr/core/application.py

+        :param halt_before: _description_, defaults to None
+        :param inputs: _description_, defaults to None
+        :return: _description_
+        :yield: _description_


skrawcz · 2025-04-28T00:03:47Z

tests/core/test_application.py

+    assert len(stream_event_tracker.pre_start_stream_calls) == 2
+    assert len(stream_event_tracker.post_end_stream_calls) == 2
+    assert len(stream_event_tracker.post_stream_item_calls) == 20
+    assert len(stream_event_tracker.post_stream_item_calls) == 20


elijahbenizzy force-pushed the stream-iterate branch from 4ba7efd to 464712c Compare April 19, 2025 21:43

skrawcz reviewed Apr 20, 2025

View reviewed changes

burr/core/application.py Outdated Show resolved Hide resolved

Adds API for streaming and iterating at the same time

ff9564b

This allows us to iterate over streaming outputs. E.G. for deep-research mode. See #360

elijahbenizzy force-pushed the stream-iterate branch from 464712c to ff9564b Compare April 20, 2025 16:56

elijahbenizzy mentioned this pull request Apr 20, 2025

Parallel Streaming constructs #534

Open

elijahbenizzy marked this pull request as ready for review April 27, 2025 17:12

ellipsis-dev bot reviewed Apr 27, 2025

View reviewed changes

skrawcz approved these changes Apr 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds API for streaming and iterating at the same time #532

Adds API for streaming and iterating at the same time #532

elijahbenizzy commented Apr 19, 2025 •

edited by ellipsis-dev bot

Loading

github-actions bot commented Apr 19, 2025 •

edited

Loading

elijahbenizzy commented Apr 21, 2025

ellipsis-dev bot left a comment

ellipsis-dev bot Apr 27, 2025

skrawcz Apr 27, 2025

ellipsis-dev bot Apr 27, 2025

skrawcz Apr 27, 2025

skrawcz left a comment

skrawcz Apr 27, 2025

skrawcz Apr 27, 2025

skrawcz Apr 28, 2025

	:param halt_after: Action names/tags to halt before
	:param halt_after: Action names/tags to halt after the action completes

Adds API for streaming and iterating at the same time #532

Are you sure you want to change the base?

Adds API for streaming and iterating at the same time #532

Conversation

elijahbenizzy commented Apr 19, 2025 • edited by ellipsis-dev bot Loading

Changes

How I tested this

Notes

Checklist

github-actions bot commented Apr 19, 2025 • edited Loading

elijahbenizzy commented Apr 21, 2025

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

ellipsis-dev bot Apr 27, 2025

Choose a reason for hiding this comment

skrawcz Apr 27, 2025

Choose a reason for hiding this comment

ellipsis-dev bot Apr 27, 2025

Choose a reason for hiding this comment

skrawcz Apr 27, 2025

Choose a reason for hiding this comment

skrawcz left a comment

Choose a reason for hiding this comment

skrawcz Apr 27, 2025

Choose a reason for hiding this comment

skrawcz Apr 27, 2025

Choose a reason for hiding this comment

skrawcz Apr 28, 2025

Choose a reason for hiding this comment

elijahbenizzy commented Apr 19, 2025 •

edited by ellipsis-dev bot

Loading

github-actions bot commented Apr 19, 2025 •

edited

Loading