Transition Logic in Chatsky

[alimaskin]

I've noticed that in the get_next_label function in chatsky/core/transition.py, all destination coroutines are executed in parallel using asyncio.gather(), but only the first valid result is actually used:

destination_results = await asyncio.gather(
    *[transition.dst.wrapped_call(ctx) for transition, _ in transitions_with_priorities]
)

for destination in destination_results:
    if isinstance(destination, AbsoluteNodeLabel):
        return destination
return None

This approach seems inefficient and can lead to errors in logs. For example, when using a Backward() destination from the first node in a flow, I get an error:

ERROR:chatsky.core.script_function:An exception occurred in Backward.
...
IndexError: Node index -1 out of range for node_keys: ['q1', 'q2', 'q3', 'thank_you', 'confirm_stop']. Consider using the `loop` flag.

Even though the system correctly chooses a higher priority transition, it still attempts to execute all transitions, including those that would fail.

Questions

1. What is the rationale behind executing all destination coroutines in parallel when only the first valid result is used?
2. Wouldn't it be more efficient to execute only the highest priority transition first, and only try lower priority transitions if the higher ones fail?
3. Is there a way to prevent these errors in logs without modifying the library code (other than using loop=True or removing problematic transitions)?

[RLKRo]

Hello!
Sorry for the long reply, I was receiving notifications from issues and PRs only, and only now noticed this discussion.

What is the rationale behind executing all destination coroutines in parallel when only the first valid result is used?
Wouldn't it be more efficient to execute only the highest priority transition first, and only try lower priority transitions if the higher ones fail?

The main reason is consistency with how conditions and priorities are executed.

I agree that in most cases it would be more efficient to execute destinations consecutively since it's not likely that a destination would raise an error. I'll consider adding an option to choose how destinations are executed.

Is there a way to prevent these errors in logs without modifying the library code (other than using loop=True or removing problematic transitions)?

Yes, you can use logging.Filter. Here's an example:

import logging

from chatsky.destinations import Backward
from chatsky import Pipeline, TRANSITIONS, Tr, RESPONSE


pipeline = Pipeline(
    script={
        "flow": {
            "node1": {
                TRANSITIONS: [Tr(dst="node2"), Tr(dst=Backward())]
            },
            "node2": {RESPONSE: "Hi"}
        }
    },
    start_label=("flow", "node1"),
    fallback_label=("flow", "node2"),
)


class ExcFilter(logging.Filter):
    def __init__(self, msg):
        super().__init__()
        self.msg = msg

    def filter(self, record):
        if record.exc_info:
            return self.msg not in str(record.exc_info[1])
        return True


logger = logging.getLogger("chatsky")

handler = logging.StreamHandler()
handler.addFilter(ExcFilter("Consider using the `loop` flag."))

logger.addHandler(handler)


if __name__ == "__main__":
    ctx = pipeline("test")
    print(ctx.last_response)