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

refactor: Add emitter event types and fix mypy #550

Merged
merged 10 commits into from
Mar 11, 2025

Conversation

ajbozarth
Copy link
Member

@ajbozarth ajbozarth commented Mar 10, 2025

Which issue(s) does this pull-request address?

Ref #415
Ref #433

Description

This include both the first part of typing the emitter event emits and addressing mypy:

Event Type work includes

  • Adds types for each event in a events.py file for each submodule
  • Updates emitter to take in and store an optional events dict that defines the expected events for that emitter. This events dict can be dynamically updated by users as needed
  • Adds a event_data key to EventMeta that contains the data type for the event
  • Add a events dict to each emitter instance with it's correct events and data types
  • Update all internal event names to use underscores not camelCase
  • Reintroduce regex support in emitter match

Event Types work TODOs

  • Update internal emits to return the new event classes instead of dicts
  • Update examples and tests to handle new event types
  • Update docs

MyPy updates

  • mypy passes on everything in beeai_framework/ except for beeai_framework/adapters and beeai_framework/memory
  • I had to add ignore lines in a few places where fixing the mypy warn would change how the code works
  • Most updates deal with adding type hints or type params for generics
  • Updated the generic names in Tool to match TS
  • Added AnyTool, AnyAgent and AnyMessage
  • created types.py for the backend, tools, and workflows submodules and move the relevant classes into those files

Checklist

General

Code quality checks

  • Linting passes: Python poe lint or poe lint --fix / TypeScript yarn lint or yarn lint:fix
  • Formatting is applied: Python poe format or poe format --fix / TypeScript: yarn format or yarn format:fix
  • Static type checks pass: Python poe type-check

Testing

  • Unit tests pass: Python poe test --type unit / TypeScript yarn test:unit
  • E2E tests pass: Python poe test --type e2e / TypeScript: yarn test:e2e
  • Integration tests pass: Python poe test --type integration
  • Tests are included (for bug fixes or new features)

Documentation

  • Documentation is updated
  • Embedme embeds code examples in docs. To update after edits, run: Python poe docs --type build

@ajbozarth ajbozarth added the python Python related functionality label Mar 10, 2025
@ajbozarth ajbozarth self-assigned this Mar 10, 2025
@ajbozarth ajbozarth requested a review from a team as a code owner March 10, 2025 22:17
@ajbozarth ajbozarth mentioned this pull request Mar 10, 2025
5 tasks
@ajbozarth
Copy link
Member Author

While working #415 I started addressing a lot of mypy issues as they came and I found that the majority of my changes had become mypy fixes. As such I decided to push what I have at this point since my event types work done so far is currently backwards compatible and could be merged as is.

My next step is to start updating the actual emit call to return the typed data objects rather than dicts and to update any code that listens on those objects. This step will be the breaking change. I will continue work on this branch unless we decide to merge it as is and I'll continue my work in a follow up PR

@ajbozarth
Copy link
Member Author

Inspired by my review of #551 I updated some more mypy and added AnyTool as in #551 and well as AnyAgent

Copy link
Contributor

@Tomas2D Tomas2D left a comment

Choose a reason for hiding this comment

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

  • I would consider extracting event classes to events.py per submodule if possible.
  • Message[Any] could be replaced with a type alias AnyMessage
  • Changing event names is a breaking change -> should be documented (mentioned somewhere).

@ajbozarth
Copy link
Member Author

Followup tasks from review:

  • extracting event classes to events.py per submodule
  • You can set default events in base _create_emitter method, but user should be able to extend them (not overwrite)

BREAKING CHANGE: rename "newToken" event to "new_token"

Signed-off-by: Alex Bozarth <[email protected]>
@ajbozarth
Copy link
Member Author

I've pushed the updates addressing the review items I did not mention in my above comment to handle in a followup

@ajbozarth
Copy link
Member Author

While waiting for review I took a stab at "extracting event classes to events.py per submodule" but hit an issue. It was actually tribal to do so for agents, but for chat, tool, and workflow it causes circular inputs due to the lack of type.py files in those submodules to hold the common types.

I can create a types.py file for backend, tools, and workflows if that's what we want though @Tomas2D

@ajbozarth
Copy link
Member Author

I was also able to address users extending emitter events dict rather than overwriting it, so no more followup PR work from review remaining

@ajbozarth
Copy link
Member Author

So after some work leveraging PyCharm's "Refactor > Move File" feature I created the types.py file for backend/, tools/, and workflows/ which then allowed me to also create events.py for those submodules as well. Make this fully address al previous review

@ajbozarth ajbozarth enabled auto-merge (squash) March 11, 2025 18:58
@ajbozarth
Copy link
Member Author

FYI I've enabled auto-merge with the requested merge commit message (BREAKING CHANGE)

@Tomas2D Tomas2D disabled auto-merge March 11, 2025 21:39
Tomas2D
Tomas2D previously approved these changes Mar 11, 2025
Copy link
Contributor

@Tomas2D Tomas2D left a comment

Choose a reason for hiding this comment

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

Well done. I like it, just the one event name and feel free to merge.

@ajbozarth ajbozarth merged commit 8e5f490 into i-am-bee:main Mar 11, 2025
4 checks passed
@ajbozarth ajbozarth deleted the event-types branch March 11, 2025 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Python related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants