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

added Select to define transitions #361

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

added Select to define transitions #361

wants to merge 1 commit into from

Conversation

zilto
Copy link
Collaborator

@zilto zilto commented Sep 9, 2024

Following #359, this PR introduces Select as an alternative to Condition when defining "one to many" transitions.

Not only can this be more ergonomic for developers, it allows to resolve multiple conditions at once. This was impossible before and conditions were only binary checks evaluated sequentially.

For example, this is now a valid Graph definition

import random
from burr.core import action, ApplicationBuilder, State, Select, Action

def random_decider(state: State, actions: list[Action]) -> str:
    action_idx = random.randint(0, len(actions)-1)
    return actions[action_idx].name

app = (
    ApplicationBuilder()
    .with_actions(
        process_user_input, generate_text, generate_image,
        ask_user_for_details, send_response
    )
    .with_transitions(
        (
            "process_user_input",
            ("generate_text", "generate_image", "ask_user_for_details"),
            Select(["user_query"], resolver=random_decider),
        ),
        (
            ("generate_text", "generate_image", "ask_user_for_details"),
            "send_response"
        ),
        ("send_response", "process_user_input"),
    )
    .with_entrypoint("process_user_input")
    .build()
)

decide

Changes

  • Added the Select construct
  • Updated the .get_next_node() and .with_transitions() of Graph and GraphBuilder to support Select
  • Updated the GraphBuilder() and ApplicationBuilder() type annotations to receive Select
  • Updated the GraphBuilder() and ApplicationBuilder() type annotations to use Sequence instead of List

How I tested this

  • Added two tests to create Select objects and call the .run() method
  • all previous tests are still passing

Potential TODOs

  • add a default kwarg to Select. This could be the default Action to use if the resolver returns None for instance
  • Allow the resolver to directly return an Action or an integer index of the action instead of action_name: str
  • Properly type the Select.resolver because it currently dislikes user-provided functions annotated with list instead of Sequence

TODOs

  • Add Select to the main docs
  • Add docstring documentation with examples (where exactly? directly on Select? Should I update .with_transitions() of other objects?)
  • Create an examples/ that uses an LLM to decide the next action (see 1-to-many transitions #359)

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.

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.

1 participant