Add basic bot framework

[Kritzefitz]

Since there doesn't seem to be good framework for writing Matrix bots in Haskell, I thought I'd contribute some basic functionality to this one. I hope it's at least somewhat along the lines of what you imagined.

The features I'm planning to implement in this PR are mostly oriented around what I need at the moment. This includes:

• [x] flexible asynchronous handling of events

Some things to consider as extensions after this PR is merged:

• [ ] Tracking membership in rooms
• [ ] Read receipts

[TristanCacqueray]

That's amazing, thank you for taking the time to contribute this work. I'm not super familiar with the monad-control and the extensions you put together for the Bot.State module. So my only concern is that this may be too advanced, and I meant to keep this project as accessible as possible, for example by adding a Tutorial module.

That being said, I think the API you proposed looks great. If I understand correctly, the user just need to provide the BotEventHandler, and the matrix-bot will call the handler with the events asynchronously. I assume the monad-control is a necessary evil to make this work while supporting user defined state for the handler. I wonder if a similar implementation can be done with RIO/unliftio, but since you already put this one together, I am happy to build on your implementation.

Thus it would be good to demonstrate a slightly more complete example, for example an handler that replies to !ping message with a pong X reply where X is a message counter.

[TristanCacqueray]

Also, my original plan for that matrix-bot project was to re-implement the lambdabot (that is to evaluate user provided lambda expression and reply with the result).

Then leveraging m.relates_to and m.new_content event I was thinking the bot could update its replies when the user edit the code, for example to fix a typo. And finally, since we can query the room history, perhaps the bot could keep in scope the recent expression, in order to enable some sort of collaborative programing.

Here is proof of concept:

alice> let caps = map toUpper
↳ bot> caps :: String -> String

bob  > caps hello
↳ bot> Variable not in scope: hello

[bob edit the message]
bob  > caps "hello"
↳ bot> "HELLO"

Then as a crazy idea, the bot could consider matrix room as haskell module so that one room can import expression defined in other room =)

[Kritzefitz]

I'm not too happy about monad-control either. I mostly included it, because Control.Concurrent.Async.Lifted needs it. Using the simple unllifted Control.Concurrent.Async would also be possible, but it would be kinda tedious in some places. Ideally I would like to hide the usage of MonadControl completely. The user is not supposed to care about it. But I'm not quite there yet.

The API isn't quite complete yet, and some ideas I have floating around in my head aren't visible at all in the current code. But in the end I want a typical bot to look somewhat like this:

bot session = matrixBot session $
  -- Here is the place to do some global initialization first
  globalRoomState <- initializeGlobalRoomState
  -- start syncPoll with some bookkeeping
  startSync (eventRouter globalRoomSate)

-- A router inspects a message and decides which sync group should handle it. See below for an explanation of what sync groups are.
eventRouter globalRoomState = withReadReceipts $ do
  globalRoomStateTracking globalRoomState
  perRoomRouting perRoomHandler

-- Here would be the place to respond to specific events
perRoomHandler = _

One core concept I'm currently trying to implement, which doesn't seem to be all too common among chatbot SDKs, are sync groups. The problem I'm trying to solve with them is that you usually want messages to be handled asynchronously as much as possible, but sometimes you might still want some synchronicity between different messages. For example, you might want to handle all messages in a given room in the correct order, but you don't care about the ordering between messages in different rooms. Simply said, a sync group is a thread that reacts to events. The router (named eventRouter above) decides for each message which sync group(s) have to process that message and which don't. I know, this might sound a bit complicated, but I hope to implement an API for this, that makes this as easy as just picking some pre-defined async behavior so the user doesn't have to care about it too much.

One other major point that I still need to do some experimenting with is state keeping. As far as I identified it now, there are three „levels“ at which the user might keep some kind of long-lived state:

• There might be state over the whole lifetime and all parts of the bot.
• The router should be able to locally keep state across multiple events, for example to remember which async group handles which rooms.
• Long-lived async group should be able to keep local state.

I'm not sure for all places on how to organize the transformer stack to make it easy for the user to just insert the right transformers in the right places to get their state managed automatically.

[TristanCacqueray]

If I understand correctly, UnliftIO.Async would also work, and it's perhaps more beginner friendly. I think it's fair to require the user provided MatrixBotBase to implement MonadUnliftIO, especially if that makes the whole implementation simpler. Like that user code has an easy path to handle exception handling, or to use any other existing unliftio facilities.

Your sync groups idea makes a lot of sense to me, it sounds like a good compromise between full sync and per events async. I don't think this too complicated and that's worth pursuing.

Then about transformer stacks, would it be possible to avoid it in the public API? For example, inspired by the Brick.Main.App could the bot be defined as something like:

data Bot global_state room_state m =
  Bot {
  , initGlobalState :: m global_state
  , handleGlobalEvent :: global_state -> GlobalEvent -> m global_state 
  , initRoomState :: RoomID -> m room_state
  , handleRoomEvents :: global_state -> room_state -> RoomID -> [RoomEvent] ->  m room_state
  }

data GlobalEvent = Invite RoomID | GlobalEvent RoomEvent

I like this design because the state management is explicit, the handlers just produce new state, and there is no need to fiddle with lifting to use the state.

[TristanCacqueray]

I misread your proposal which suggest custom event groups. So here is another draft:

data Bot state m =
  Bot {
  , initState :: m state
  , handleEvent :: state -> BotEvent -> m state
  }

And to support sync groups, the matrix-bot could provide such facility:

type EventGroup s = (s, TBMQueue BotEvent)
eventRouter ::
  -- | The event to dispatch 
  BotEvent ->
  -- | The queue and its state
  (EventGroup s)
  -- | The async handler
  (s -> [BotEvent] -> m ()) ->
  m ()
eventRouter event queue handler = _

mkEventGroup :: s -> EventGroup s
mkEventGroup = _ -- newTBMQueueIO

So that an async bot (for example per room) could be written as:

type GroupState = Map RoomID (EventGroup s)
botHandleEvent :: GroupState -> BotEvent -> m GroupState
botHandleEvent glState event = do
  (eventGroup, glState) <- getGroup event
  eventRouter event eventGroup handleGroupEvents
  pure glState
  where
    getGroup = case Map.lookup (room ev) glState of
      Just roomState -> pure (roomState, glState)
      Nothing -> do
        roomState <- mkGroup =<< newGroupState (room ev)
        pure (roomState, Map.insert (room ev) roomState glState)

handleGroupEvent :: groupState -> [BotEvent] -> m ()
handleGroupEvent = _

Well perhaps I misunderstood the problem entirely, then nevermind my suggestions =)

[Kritzefitz]

After sleeping over this for a night, I definitely agree, that we should switch to UnliftIO.Async. It not only does the job, but it also doesn't promise to do things that it cant. Specifically the current solution will probably break in some way, if the monad involved is a state monad, because the state will be cloned on every async invocation.

As for the state handling, I thought about handling user state without explicit transformer stacks. In principle I like the idea, because it is really easy to understand and reason about. But there is one thing I don like about it: It doesn't seem very extendable. In my example above, I used two different calls to initializeGlobalRoomState and globalRoomStateTracking. The idea here is that initializeGlobalRoomState initialized (e.g.) an MVar that keeps the room state and globalRoomStateTracking updates it, when appropriate events are received. The user can then use function to query the state in that MVar (the MVar can of course be hidden behind a wrapper type, but the principle stays the same). But this requires the user to keep the reference to the roomState around explicitly. With your state keeping proposal it would mean that the user would have to manually create a field for the room state in their own user state and extract it from there every time they need it. I don't like this, because enabling room state tracking should be as easy as flipping a switch. So instead I imagined something like withGlobalRoomState :: (forall n. (HasRoomState n) => n a) -> m a where everything in the inner block would have access to the room state via the HasRoomState constraint.

But this brings up some other problems, withGlobalRoomState basically needs to do two things: Set up an appropriate monad transformer for the state keeping and arrange that the appropriate handlers that update the state are called for every event. This becomes a bit more complex, because if we allow modifications to the transformer stack during the initialization of the router, the transformer stack of the actual event handling function might be different from the transformer stack withGlobalRoomState was called on. I have some ideas on how to handle it, but ultimately I will have to try what works. But overall I agree that the API will probably be easier if we don't let the user fiddle around with monad transformers explicitly.

Your draft looks like you got the right idea, but I imagine the terminology to be somewhat different from how you applied it. The part you named botEventHandler is the router, so the user would probably call it something like botEventRouter. What you named eventRouter isn't actually “the router”, but just one utility used in the construction of routers, so I would have named it something like passToSyncGroup.

One functional difference I see is that you allow a new event handler to be passed along with eventRouter every time it is called, while I imagined that the behavior of a EventGroup would have be passed in the call to mkEventGroup. This would allow the user to define SyncGroups whose handlers aren't simple loop constructs. But I guess the most efficient way to show, would be to make a working draft in the actual code.

[TristanCacqueray]

Thank you for taking the time to write the details, I think we are making good progress here.

I am happy to use a different terminology and API, and it is likely to evolve as we get to use it with actual code :)

Another design that might be worth looking into is the keid Engine.Types.Stage datatype. It is using two contexts StageRIO and StageFrameRIO that looks similar to our use case. So if such design could work for our API, then I think we should go for it and build on top of rio as it provides a solid and opinionated foundation.

[Kritzefitz]

I just pushed a new commit. It streamlines the state keeping a bit and implements routing. The idea for state keeping is now, that the user can modify the base transformer stack, before sync starts, and it is used everywhere else. This base transformer stack needs to implement MonadUnliftIO, so we can pass it to other threads.

The state keeping in the router became much more involved. Basically, a router always is a state monad, where the user can store their custom state. As I mentioned earlier, I wanted to make the state management a bit more involved, so certain “plugin” parts of the router can store their state without interfering with the users state. This is achieved by allowing the transformer stack of the router to be extended using transformRouter. This works, but isn't really for the faint of heart. But I imagine that users won't have to deal with the most complex bits of this. Mostly they will either deal with a pre-made plugin (e.g. withRoomTracking, not implemented yet) or use withExtraState to get access to simple MonadState methods to manage their own state.

After having implemented this, I'm afraid that this might still be a bit too complex to easily understand. But I first wanted to finish something that works and has the properties I want, so I can reason about it. I hope, once I can mull over this for a bit and try out how this looks from the user perspective under practical circumstances, I will have a clearer understanding on how to improve this and hide the complex bits between easy-to-understand utility functions.

[Kritzefitz]

I just noticed that my current solution to state handling in the router is rather overcomplicated. I thought that I would need to squash all state of the user and plugins (by plugins I mean functions that modify an existing router to add commonly needed functionality) into one state monad. This became overly complicated, when I tried to streamline the access to individual bits of the shared state.

I started following this squashed state approach, because I didn't see an easy way to allow the user to extend the transformer stack of the router using regular tools. But I just now I realized, that I can re-use the approach used in MatrixBotOptions to pass a run function for the added transformer stack along with the code to be executed. So basically BotEventRouter would then look like this:

data BotEventRouter m n where
  BotEventRouter :: { berRunCustomStack :: forall a. m a -> n a
                    , berDoRoute :: BotEvent -> m ()
                    } -> BotEventRouter s m n

and the type of transformRouter would become

transformRouter :: (forall a. m' a -> m a) -> BotEventRouter m' n -> BotEventRouter m n

But I definitely spent enough time with this for today, so I'm gonna pick this up some other time, so stay tuned.

[TristanCacqueray]

It's definitely a clever, and ambitious path you are taking, so it's ok to keep the complicated part around until we get something ergonomic.

If I understand correctly, the main challenge is to handle multiple state monads which requires type level machinery to please ghc. I don't think that's necessarily a bad thing, and the end-user API could make it up for it... I'm looking forward to see its usage in practice.

[Kritzefitz]

I just pushed a new commit that replaces the previous state keeping in the router with hoistRouter that allows the user to manage their own transformer stack in the router. I'm kinda happy with how it works, but I think it seems kinda complex. As it is now, there are different overlapping transformer stacks in various places, and I'm afraid the user will have to at least have a basic understanding of the structure to inject their own transformers into the stack.

Currently the stack is somewhat structured like this:

1. IO (of course)
2. MatrixBot provides access to the ClientSession and some other important values, but is hidden behind mtl-style behind type classes.
3. The users own transformers introduced through runBotT

Throughout the codebase I try to call this the “base transformer stack” or similar. It is notably set apart, because all parts of the bot have it at the bottom of their transformer stack (i.e. bot the router and the event handlers have access to it).

Inside the router these additional transformers are layered on top of it:

4. SyncGroupManager, needs to be below the router state otherwise we would restrict the router state to MonadUnliftIO, hidden behind mtl-style classes
5. The users transformers for router state
6. RouterM, hidden behind mtl-style classes (but still explicitly identified as MonadTrans on top of the users transformers, so kinda awkward in mtl-style)

This means that users can access functionality of 3. through liftBotBase and 5. through lift.

Async event handlers are a bit easier in this regard. They run directly on to of the base transformer stack by default. asyncHandler allows the user to extend the transformer stack of an event handler. If they do so, it is up to them to ascertain that they have access to the base transformer stack (if required), either by lifting through their own stack explicitly, or implementing HasMatrixBotBaseLevel.

I haven't done a lot of practical testing on top of this architecture. But if it works acceptably, I guess this may close the chapter state keeping for the moment. During the process I happened to implement enough of SyncGroups to make them usable already.

What do you think should be the next steps from here? As I mentioned in the original text of the pull request, I intend to also provide plugins for room membership tracking and read receipts. Would you prefer I implement them in this PR or should I first bring the current state into shape (documentation and sorting the external API) so we have so we can merge the basic functionality and merge the extensions later?

[Kritzefitz]

We probably also need some utility functions to match on BotEvents , so we can easily write event handlers and routers that only react to certain types of events. But I think this would also be a candidate for another PR.

[TristanCacqueray]

Well I also think think this is kinda complex, but as the dear-imgui.hs maintainers once said, I'm ultimately just interested in progress, so whatever enables you to get stuff done is pretty good in my books!

I think the next step would be to get some sort of a demo, for example by promoting the MatrixBot.Main into a standalone matrix-echobot executable which demonstrate the most basic use case, e.g. synchronous reply to messages.

Though I am happy to merge this as-is and do follow-ups instead, as long as you are comfortable with the API evolving as necessary to address the un-expected use case. For example I'd like to start implementing the lambdabot I mentioned earlier with what you proposed, but I don't have time for that at the moment. So instead, I gave you the commit bit on this project so that you can merge the PR when you feel like it is ready.

One last thing, would you mind using a code formatter? If so, please do a pass on the code (e.g. find src/ app/ test/ -name "*.hs" | xargs ormolu --mode=inplace). If not that's ok too.

[Kritzefitz]

Thanks for you feedback! I don't have the time right now to go over every comment in detail, but I do find them very helpful.

Just to give you an idea of the things I consider to still be necessary (on top of the things you noted in this and previews reviews):

• [ ] Haddock documentation, at least for the public API
• [ ] A better example bot than MatrixBot.Main. For example, as you suggested, a simple echo bot.

[TristanCacqueray]

@Kritzefitz have you seen https://github.com/cofree-coffee/cofree-bot/ ?

[Kritzefitz]

I had not. At first glance it seems really promising. Though the lack of documentation worries me slightly.

As for the state of this pull request, unless cofree-bot turns out to be a more mature and usable alternative, I'm still interested in moving this forward. Unfortunately I didn't have the time and energy required to actually do so. I also don't see that this will magically change in the near future.

However, I don't want to block progress on this, if you or anyone else wants to work on this. So feel free to continue working on this, or merge as-is (if you feel comfortable with the current state) or just wait until I continue working on this. Whatever you're most comfortable with.

[solomon-b]

I had not. At first glance it seems really promising. Though the lack of documentation worries me slightly.

FYI, Cofree Bot is a project a few friends and I are working on. It takes a very different approach from this PR and is still in a very experimental state. I don't see why one project should preclude the other.

[TristanCacqueray]

Thank you for proposing that implementation. Though I'm afraid this goes beyond the scope of this project, and such matrix-bot should probably live in another repository.

I'm closing this for now, but feel free to re-open if you think otherwise.