Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New streaming events, EventListener listen on parent types #1266

Merged
merged 5 commits into from
Oct 21, 2024

Conversation

vachillo
Copy link
Member

@vachillo vachillo commented Oct 17, 2024

Describe your changes

  • Allow EventListener to listen on parent types.
  • Create new BaseChunkEvent
  • Add ActionChunkEvent and TextChunkEvent
  • remove CompletionChunkEvent

Issue ticket number and link


📚 Documentation preview 📚: https://griptape--1266.org.readthedocs.build//1266/

Copy link

codecov bot commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 94.73684% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
griptape/utils/stream.py 78.57% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@vachillo vachillo force-pushed the stream_meta branch 2 times, most recently from c88058c to 310e8e0 Compare October 17, 2024 17:58
@vachillo vachillo marked this pull request as ready for review October 17, 2024 17:58
@vachillo vachillo changed the title Differentiate between streaming events New streaming events, EventListener listen on parent types Oct 17, 2024
@vachillo vachillo requested a review from collindutter October 17, 2024 19:33
docs/griptape-framework/misc/src/events_3.py Outdated Show resolved Hide resolved
griptape/events/action_chunk_event.py Outdated Show resolved Hide resolved
griptape/drivers/prompt/base_prompt_driver.py Outdated Show resolved Hide resolved
griptape/events/base_chunk_event.py Outdated Show resolved Hide resolved
griptape/events/event_listener.py Show resolved Hide resolved
griptape/events/text_chunk_event.py Outdated Show resolved Hide resolved
Comment on lines 7 to 11
EventBus.add_event_listeners(
[
EventListener(
lambda e: print(cast(CompletionChunkEvent, e).token, end="", flush=True),
event_types=[CompletionChunkEvent],
lambda e: print(e.token, end="", flush=True),
event_types=[TextChunkEvent],
Copy link
Member

Choose a reason for hiding this comment

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

Should update this to include ActionChunkEvents as well since the agent uses Tools.

griptape/events/action_chunk_event.py Outdated Show resolved Hide resolved
griptape/utils/stream.py Outdated Show resolved Hide resolved
Comment on lines 76 to 77
yield TextArtifact(value=json.dumps(json.loads(action_str), indent=2))
action_str = ": "
Copy link
Member

Choose a reason for hiding this comment

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

Let's print the body on a new line instead of same line with : . More consistent with non-streaming output.

Comment on lines 75 to 76
json.loads(action_str)
yield TextArtifact(value=json.dumps(json.loads(action_str), indent=2))
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessarily parsing twice.

Makefile Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Nice

Comment on lines 8 to 18
def action_chunk_listener(event: ActionChunkEvent) -> None:
if event.tag is not None and event.name is not None and event.path is not None:
print(f"{event.name}.{event.tag} ({event.path}) ", end="", flush=True)
if event.partial_input is not None:
print(event.partial_input, end="", flush=True)


EventBus.add_event_listeners(
[
EventListener(
lambda e: print(e.token, end="", flush=True),
event_types=[TextChunkEvent],
),
EventListener(
action_chunk_listener,
event_types=[ActionChunkEvent],
),
]
)
Copy link
Member

Choose a reason for hiding this comment

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

Can this be simplified now that we have __str__ on the events?

Copy link
Member Author

Choose a reason for hiding this comment

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

events_3.py is the simplified version of this. this is an example showing how to listen on the sub event types

Copy link
Member

Choose a reason for hiding this comment

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

We can still show listening on sub-types while using the str method right?

Copy link
Member Author

Choose a reason for hiding this comment

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

tis possible

Comment on lines +10 to +11
def __str__(self) -> str:
return self.token
Copy link
Member

Choose a reason for hiding this comment

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

Test pls.

collindutter
collindutter previously approved these changes Oct 21, 2024
Copy link
Member

@collindutter collindutter left a comment

Choose a reason for hiding this comment

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

Nice

@collindutter collindutter merged commit dab865a into dev Oct 21, 2024
15 checks passed
@collindutter collindutter deleted the stream_meta branch October 21, 2024 23:27
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.

2 participants