A Good Idea?

Our message format between Bot and World is a naked JSON structure. That makes sense for messaging, but I don’t like it for processing. I have an idea that I think will help.

The message form is a list of dictionaries. Each dictionary has a key ‘entity’, which is to point at an entity id, and a key ‘actions’, which points to a list of dictionaries, each of which has a key ‘verb’ and a key ‘param1’ (and conceivably other parameter keys later). We have a nice object, currently named RequestBuilder, which will soon come into play in production, for building these messages. I am not sure about the class name, but we can change those readily, thanks to PyCharm’s cleverness.

The code that processes one of these messages looks like this:

class World:
    def process(self, world_input):
        output = []
        for request in world_input:
            requestor_id = request['entity']
            self.execute(request)
            output.append(self.fetch(requestor_id))
        return output

    def execute(self, request):
        id = request["entity"]
        entity = self.entity_from_id(id)
        actions = request["actions"]
        for action in actions:
            self.execute_action(entity, action)

    def execute_action(self, entity, action):
        verb = action["verb"]
        match verb:
            case 'step':
                self.step(entity)
            case 'take':
                self.take_forward(entity)
            case 'drop':
                holding_id = action["param1"]
                holding = self.entity_from_id(holding_id)
                self.drop_forward(entity, holding)
            case 'turn':
                direction = action["param1"]
                self.set_direction(entity, direction)
            case _:
                raise Exception(f'Unknown action {action}')

This code is pretty fragile, in that if any of those keys is missing, it will raise an exception and terminate the World. That would be bad. This is consistent with my usual experience using native collections, even in far less structured forms than this one. I think we would benefit from some objects built specifically to deal with the message structure in use, in a sense parallel to the RequestBuilder. This one might be a RequestProcessor. And we might want to name each of them MessageSomething instead of RequestSomething.

I propose to spike this idea, in the Jeffries format, which is to say that I am prepared to throw the code away, but if I like how it is shaping up, I will retain and improve it. Spike aficionados the world around will tell you that you have to throw away a spike. I’m like, OK, whatevs.

First things first, let’s move the RequestBuilder to its own file, in the client folder. Thanks PyCharm.

Let’s start our spike, with TDD, of course. I’ll create a new test file.

class TestRequestProcessor:
    def test_hookup(self):
        assert False

Fails. I should look into creating a macro or template or whatever they are for that. But it’s easy to type.

What I have in mind for RequestProcessor is that it will be made up of a few small classes. The top level one, RequestProcessor, will be given a message in the form described above, a list of dictionaries of lists of dictionaries.

That class may do some elementary checking, essentially to see that it has been passed a list, or whatever the form of the top level of our message is. It’s a list for now. (Note that RequestBuilder and RequestProcessors are “dual”, they need to match each other, since one builds the thing and the other interprets it.)

The Processor will have an iteration method that will iterate the provided list and return each element wrapped in another class, which will contain an individual request. (It seems that that class may better deserve the name RequestProcessor. We’ll see.) Whatever it’s called, this one will have a method to fetch the ‘entity’ id, and will be able to iterate the ‘actions’, executing them. I figure, vaguely, through a glass darkly, that these objects will be called from the World, approximately where the process method is now, and that they’ll subsume the process, execute and execute_action methods.

Mind you, this is speculation. The code will tell us what we need. We’ll begin with some tests that just tell us more about how to structure these objects.

Aside

I forgot to mention that I have another vague idea, about error conditions. I’m thinking that when the World detects an error or even raises an exception. We’ll add an ‘error’ string to the Knowledge we return. So that is on the edge of my mind, but I’ve decided to defer doing it until we have an error to deal with and this Processor idea either pans out or fizzles out.

Let’s test this baby. I’ll need a message to process.

    def message(self):
        builder = RequestBuilder()
        result = builder \
            .request(101) \
                .action('step') \
                .action('take') \
            .request(102) \
                .action('drop', 103) \
                .action('turn', 'EAST') \
                .action('step') \
            .result()
        return result

    def test_message(self):
        message = self.message()
        assert message == ''

This fails and shows me what I’ve built:

[{'actions': [{'param1': None, 'verb': 'step'},
              {'param1': None, 'verb': 'take'}],
  'entity': 101},
 {'actions': [{'param1': 103, 'verb': 'drop'},
              {'param1': 'EAST', 'verb': 'turn'},
              {'param1': None, 'verb': 'step'}],
  'entity': 102}] 

PyCharm formats it nicely. I am always a bit thrown by the fact that it alphabetizes the keys but I am grateful for the nice formatting.

So let’s see what we can actually test. First, the top level object, which will basically iterate the list.

    def test_iteration(self):
        message = self.message()
        processor = MessageProcessor(message)
        count = 0
        for rq in processor:
            count += 1
        assert count == 2

For this, we just need this:

class MessageProcessor:
    def __init__(self, message: list):
        self.message = message

    def __iter__(self):
        for rq in self.message:
            yield rq

And you can see that I’ve decided to name this class in honor of the message. The message is a list of requests.

When I was defining __iter__, PyCharm guessed that I was going to define process. As you can see with __iter__, I was thinking that the Processor acts like a list of something, requests probably, that we can iterate. But instead, we could pass the World to it, presumably from within World itself, and have it process things by calling back to World. Let’s take a closer look at how we do that now:

    def execute(self, request):
        id = request["entity"]
        entity = self.entity_from_id(id)
        actions = request["actions"]
        for action in actions:
            self.execute_action(entity, action)

    def execute_action(self, entity, action):
        verb = action["verb"]
        match verb:
            case 'step':
                self.step(entity)
            case 'take':
                self.take_forward(entity)
            case 'drop':
                holding_id = action["param1"]
                holding = self.entity_from_id(holding_id)
                self.drop_forward(entity, holding)
            case 'turn':
                direction = action["param1"]
                self.set_direction(entity, direction)
            case _:
                raise Exception(f'Unknown action {action}')

    def step(self, bot):
        self.map.attempt_move(bot.id, bot.forward_location())  # changes world version
        self.set_bot_vision(bot)
        self.set_bot_scent(bot)

    def set_bot_vision(self, bot):
        bot.vision = self.map.vision_at(bot.location)

    def set_bot_scent(self, bot):
        aroma_to_seek = 0
        if bot.holding:
            aroma_to_seek = bot.holding.aroma
        bot.scent = self.map.scent_at(bot.location, aroma_to_seek)

    def take_forward(self, bot):
        is_block = lambda e: e.name == 'B'
        if block := self.map.take_conditionally_at(bot.forward_location(), is_block):
            bot.receive(block)

That’s a lot to take in, but we’re here to scan it, not grok it in fullness. I have two issues in mind. The first is simple enough: we have individual methods on World for stepping, taking, and so on. So our processor could certainly call those methods.

My second issue is a bit more forward-looking. I’m wondering whether those detailed methods, step, take_forward, and so on, are really properly methods on World. At a glance, they seem more concerned with the map and the bot entity. Now, presently the bot entity is a pretty dumb object, just a cover for the knowledge dictionary that the World maintains. I think we might find that some of the World code suffers from Feature Envy, and that a smarter WorldEntity might improve our code. We’ll just let that idea perk.

In any case, we can certainly have our MessageProcessor call the World directly, which I think is better than having the world do the untangling that it currently does in process, execute and execute_action.

We’ll try to work that way. And therein lies the rub: how do we conveniently test an object whose behavior is to call methods on World, that do all kinds of monstrous things? This may push me into building another test double, this time a fake world.

But that’s not going to happen for a while, is it?

This is a spike. Let me sketch the process method, via Wishful Thinking, coding what I wish would work.

In an astounding flurry, I have this test:

    def test_process(self):
        message = self.message()
        processor = MessageProcessor(message)
        result = processor.process()
        assert result == []

With this result:

['for entity 101:',
 '    doing step None',
 '    doing take None',
 'for entity 102:',
 '    doing drop 103',
 '    doing turn EAST',
 '    doing step None'] != []

And that’s what I wanted to see. Here are my spike classes so far:

class ActionProcessor:
    def __init__(self, action):
        self.verb = action['verb']
        self.param1 = action['param1']

    def process(self):
        return f'    doing {self.verb} {self.param1}'


class RequestProcessor:
    def __init__(self, raw_rq):
        self.rq = raw_rq

    def actions(self):
        return self.rq['actions']

    def process(self):
        id = self.rq['entity']
        result = [f'for entity {id}:']
        for action in self.actions():
            action_processor = ActionProcessor(action)
            result.append(action_processor.process())
        return result


class MessageProcessor:
    def __init__(self, message: list):
        self.message = message

    def __iter__(self):
        for raw_rq in self.message:
            yield RequestProcessor(raw_rq)

    def process(self):
        result = []
        for rq in self:
            result.extend(rq.process())
        return result

Reflection

So the test shows that we can process all the way down. The classes themselves are not all quite the same in shape, and that troubles me. In particular, MessageProcessor and RequestProcessor are pretty similar in what they actually do, but they do it differently. The one yields a sub-processor and the other builds its sub-processor inside the for loop. We should pick a way and do it.

Another concern, of course, is that while this spike shows how we might solve the problem, it will need to be changed substantially in order to actually call the World as needed.

Finally, it’ll be at best a pain to build the result for this test, so that we can actually get it green.

N.B.

You’ll see below that it wasn’t a pain at all. What do I know?

Post-finally, I’m a little concerned that MessageProcessor knows the name of RequestProcessor, and that RequestProcessor knows the name of ActionProcessor. But they do need to know what to create as a wrapper for the next call. I think I’ll let that concern ride for now.

Post-post-finally, and related to finally, we need to be concerned with the actual result to be returned to the client. At a guess, that will be no more than a list of the dictionaries we presently return via fetch, since they do contain the entity id, which will suffice for the client to sort out how to distribute the results.

Oh, and so far we have avoided a story test that calls world. It would be nice if we could evolve these objects so that our tests do not need either a real World or a fake World in order to test the processors.

If we could get this test green, we could commit. One way would be a simpler assert. Another would be just to paste in the result of the test, having inspected it. I’ll do that.

N.B.

See? Wasn’t a pain at all.

    def test_process(self):
        message = self.message()
        processor = MessageProcessor(message)
        result = processor.process()
        assert result == [
            'for entity 101:',
             '    doing step None',
             '    doing take None',
             'for entity 102:',
             '    doing drop 103',
             '    doing turn EAST',
             '    doing step None']

I think the cool kids call that an Approval Test. I do approve. Commit: useful first cut at Processor objects.

It’s gone 1115 hours, and I started at 0920, so let’s pause here, reflect further, and sum up.

Reflective Summary

We have three small, slightly ragged objects showing how we could process the nested list-dictionary message. It seems quite clear that if the objects had the World passed into their process methods, they could pass it own down and the ActionProcessor could call the world appropriately, adopting the match statement that now appears in execute_action.

And since we can accumulate strings, we can accumulate whatever we wish, presumably WorldEntities, for their dictionaries. Or we could accumulate just the identifiers and fetch the dictionaries later, half of one six a dozen of the other. So that’s not a problem.

I wonder if there’s some way we could create the base MessageProcessor, providing some lambdas, functions to be executed in the various loops. Then our test could pass in the ones that create the list of strings, and the real thing could pass in functions that call world.

Would that actually be better than just biting the bullet and passing in a fake World in our tests? Probably not, though it would be far more clever. But clever isn’t really something to aspire to, is it?

And we’ll have the error conditions to deal with, at least to the extent of dealing with there being no actions or no verb or something. We can clearly make the objects do nothing if the information is malformed. Making them issue error messages can be dealt with later and separately, without much difficulty, I think.

Let’s review once more what the existing process method does, noting that it isn’t used in production yet.

class World:
    def process(self, world_input):
        output = []
        for request in world_input:
            requestor_id = request['entity']
            self.execute(request)
            output.append(self.fetch(requestor_id))
        return output

    def fetch(self, entity_id):
        return self.entity_from_id(entity_id).as_dictionary()

Yes, that’s just returning the knowledge dictionary for each entity processed. Easily replicated, and pretty close to what the DirectConnection will want when we get it processing a batch.

Caveat Emptor
Weird Metaphor Ahead

We have a few active pieces on the board at present, things that are coming together but are not yet all lined up and used in production. We have the process method, used only in test. We have these new objects, intended to replace process and probably the two methods it calls, execute and execute_action. (An option would be to allow it to call those two execute methods, but I am inclined to move all the capability into the new objects.)

We have DirectConnection still processing single commands at a time, packaging up single step requests. It is not using the RequestBuilder, because … the Bot’s request-generating code currently does one operation at a time with a call to DC, and Bot needs to build up a single request packet and push that through DC.

There are probably a few more active pieces that I’m forgetting.

One might think that I¹ should grab a piece, finish it, put it where it belongs, rinse, repeat until they’re all done. But instead, I’m kind of moving each one just a bit, then another, then another. We just started to bring these new Processor pieces onto the board.

I think of this situation as being a bit like a chess game, in which we slowly move our pieces around, getting them in position, until the last piece falls into place and the game is ours. So at some point, we’ll capture the World’s process method and replace it with our new piece. We’ll change the Bot’s decision process to use a request-style object.

In the end, if all goes perfectly², things will just coalesce and converge and come together and when the ripples die down, we’ll be in a better place.

Big Thought, or Just Pretension?

Our business is changing code, as the estimable GeePaw Hill puts it. I like to show how small changes, in various places, show us other places, and other opportunities, so that through continued small actions all over the program, the whole program gets better. Here, milling around in the issues of Bots deciding on lists of things to do, connections passing messages back and forth, the World managing a strange universe of Bots and Blocks and doubtless many new objects and operations, we modify first this class, then these two, then we build a new thing, then we put a new thing into place …

And the program improves in design and capability. Small changes, repeated indefinitely. This is the way.

See you next time!