Logger fixes for openhands-resolver

[malhotra5]

End-user friendly description of the problem this fixes or functionality that this introduces

• [ ] Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

---

Give a summary of what the PR does, explaining any non-trivial design decisions

This PR

1. Adds "append" flag during logger subscription in openhands/core/main.py; this behaves the same as before but allows for subscription of other callback functions before openhands run_controller is invoked.
2. Creates an async task for ALL callback functions under every EventStreamSubscriber types. Previously it only allowed to invoking the latest subscribed callback function per EventStreamSubscriber type.

These changes enable openhands-resolver to subscribe to logs. See linked issue for more details.

---

Link of any specific issues this addresses

openhands-resolver issue

[malhotra5]

I've updated this PR with solution suggested by @enyst; this allows for openhands-resolver to subscribe to events with a unique key

See updated resolver pr for final implementation

[enyst]

I've updated this PR with solution suggested by @enyst; this allows for openhands-resolver to subscribe to events with a unique key

Personally I'm fine with that. A question though: I took a look at the agent controller and it seems to me that we may have gone a bit far with sending logs to 'debug', and IMHO we will need every event once at 'info'. Also reported in slack, it's kind of difficult to know what happens on regular runs if we don't see events. So I wonder if we should we change a few back. Then when we do, will we get duplicates in the resolver? Would that be a problem?

[malhotra5]

Then when we do, will we get duplicates in the resolver? Would that be a problem?

Yup we would have duplicates; I'll close this PR in that case.

[enyst]

What do you think if we only change this action and this observation, are they enough?

They are the logging lines we display as visible as possible, with colors and other formatting tricks in console. Just to, you know, see. 😅

The reason why they vanished is that we really had way too much logging, big stuff and small, to the point it was actually difficult to read anything. So we just cleaned up the view by sending most of them to 'debug' channel. Now we can look and see if some really are necessary (or very useful) by default. WDYT? Cc: @rbren @neubig

[neubig]

I think we should probably improve the way the event stream subscriber is implemented. The fact that it was only using the most recently subscribed listener seems quite limiting

[enyst]

For the record, previous discussion on that issue.

[malhotra5]

I'm not sure if I've interpreted this correctly, but from what I understand we want the following

1. Every agent controller handles their own callback
2. No duplicate events (parent isn't logging along with its delegate)
3. Ability for other listeners to subscribe to the same event stream

Possible solution could be

1. maintain the stack approach, using the most recently subscribed listener
2. allow for a "new chain" to be created; it would track the most recently subscribed listener a. this allows for multiple listeners that track the latest delegate b. if a delegate is done, the new chain's pointer will change to track the latest delegate c. optionally allow subscriptions to a delegate that is specific levels deep

The example below has the original stack and two additional listeners. Additional listener 1 is tracking the most recent delegate and additional listen 2 only tracks the delegate that is 1 level deep. The diagram shows how additional listeners change their tracking as delegates are added and removed.

[neubig]

I haven't had time to go through all of the code super-closely, but here's an idea (maybe it's been thought of or tried already):

If we have a delegator agent and a delagatee agent, could we just have something in agentcontroller where we:

1. subscribe delegatee, unsubscribe delegator
2. delegatee agent works and finishes the subtask
3. unsubscribe delegatee, subscribe delegator

This seems like the cleanest way to me, because we can just assume that every subscribed listener gets actions as opposed to needing to worry about stacks and chains and things.

[enyst]

I think that means unsubscribing just before creating the delegate controller, and re-subscribing here and a few lines below just after closing. This way, there should be no difference who receives the events.

WDYT?

[neubig]

Yeah, that sounds right to me!

[malhotra5]

I think it could also be helpful to store an ID attribute for every subscription? This way we manage subscribing and unsubscribing without worrying about the order in which the subscriptions were made.

Reason I think this is helpful is because openhands-resolver needs to subscribe to MAIN, so there will be at least two listeners on this event type at all times.

[malhotra5]

Updates

Change 1: Emitting to every subscriber instead of most recent subscriber

Every subscriber now subscribes to events with a unique ID. An event type can have multiple subscribers and events are emitted for all of them.

This enables openhands-resolver to subscribe to the same events as its runtime.

Note: we use unique subscription IDs so that openhands-resolver can subscribe to events while still being able to handle change 2 described below. Furthermore, this allows for easier debugging and tracking of subscribers.

Change 2: Handling redundant logging

If we have a delegator agent and a delagatee agent, we avoid redundant logging via the following logic

1. subscribe delegatee, unsubscribe delegator
2. delegatee agent works and finishes the subtask
3. unsubscribe delegatee, subscribe delegator

[neubig]

I think this looks great! I'd like to go ahead and merge this in but if @rbren could take a look that'd be great too.

[neubig]

Hey @malhotra5 , I think we can merge this but we'll need to resolve merge conflicts with main now. Thanks for the contribution!

[malhotra5]

Fixed merge issues, please double check. Thanks for the discussions and feedback so far!