state.append doesn't work unless read is specified

[skrawcz]

This works when image_location_history is added to reads=

@action(reads=["current_image_location", "image_location_history"],
        writes=["current_image_caption", "image_location_history"])
def image_caption(state: State) -> Tuple[dict, State]:
    current_image = state["current_image_location"]
    result = caption_image_driver.execute([
        "generated_caption"
    ], inputs={"image_url": current_image})
    updates = {
        "current_image_caption": result["generated_caption"],
    }
    return result, state.update(**updates).append(image_location_history=current_image)

The append wipes the values in image_location_history if image_location_history is not in reads=. I.e. this doesn't not do as one expects:

@action(reads=["current_image_location"],
        writes=["current_image_caption", "image_location_history"])
def image_caption(state: State) -> Tuple[dict, State]:
    current_image = state["current_image_location"]
    result = caption_image_driver.execute([
        "generated_caption"
    ], inputs={"image_url": current_image})
    updates = {
        "current_image_caption": result["generated_caption"],
    }
    return result, state.update(**updates).append(image_location_history=current_image)

[elijahbenizzy]

Reproducible example:

from burr.core import action, State, ApplicationBuilder, default


@action(reads=["val"], writes=["list", "val"])
def append(state: State) -> tuple[dict, State]:
    val = state["val"] + 1
    result = {"val": val}
    return result, state.append(list=result["val"]).update(val=val)


app = (
    ApplicationBuilder()
    .with_state(list=[], val=0)
    .with_actions(append=append)
    .with_entrypoint("append")
    .with_transitions(
        ("append", "append", default),
    )
    .build()
)

old_state = app.state
assert old_state["list"] == []
*_, new_state = app.step()
assert new_state["list"] == [1]
*_, new_state = app.step()
assert new_state["list"] == [1, 2]

[elijahbenizzy]

Problem is in FunctionBasedAction -- run takes in just the read state, but it does both a run and a update. So when we pass in the write state (which has the full list), it gets clobbered. Meanwhile, the default append is to create an empty list, so it keeps resetting it.

Trick here is to pass in the reads and writes from state to the run item in the action... This comes from the ugliness of doing both at the same time, but I think we can specify a combination one that does both? E.G. this does both so it wants reads and writes? And then the framework passes it in?

Ideas:

1. Make it ask for both self._reads and self._writes in reads -- this is ugly because there wouldn't be defaults, and it'll have more than it needs...
2. Make it specify that its a single-step process -- handle that differently.
3. Move the logic for subsetting state into Action so this can handle it differently

My instinct says (2) as the logic feels like it should be outside (an action should say what it wants and not grab it). Will see how complicated it is.

[elijahbenizzy]

This is fixed -- see discussion/commits in 770c27f for design.