fix: Populate context_view in COMPONENT_PRE_EXECUTE payload

[araujof]

Misc PR

Type of PR

• Bug Fix
• New Feature
• Documentation
• Other

Description

• Link to Issue: Fixes fix(plugins): COMPONENT_PRE_EXECUTE payload has empty context_view #940

Summary

• Fix COMPONENT_PRE_EXECUTE delivering None for payload.context_view by passing context.as_list() when constructing the payload in aact()
• Add tests verifying the field is populated for both non-empty and empty contexts

Changes

mellea/stdlib/functional.py

• Added context_view=context.as_list() to the ComponentPreExecutePayload constructor in aact()

test/plugins/test_hook_call_sites.py

• test_component_pre_execute_payload_has_context_view — non-empty context produces a populated list
• test_component_pre_execute_empty_context_gives_empty_list — fresh context gives [], not None

Testing

• Tests added to the respective file if code was changed
• New code has 100% coverage if code as added
• Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

Test plan

• uv run pytest test/plugins/test_hook_call_sites.py -v -k component_pre_execute — all tests pass
• uv run pytest test/plugins/test_all_payloads.py -v — no regression
• uv run ruff check / ruff format — clean

Attribution

• AI coding assistants used

Related

Part of #920.

[github-actions]

The PR description has been updated. Please fill out the template for your PR to be reviewed.

[araujof]

cc: @nrfulton @jakelorocco

[psschwei]

I also had a fix for this in #921 - we can merge either PR, doesn't matter to me.
For comparison, I had gone with using context.view_for_generation() rather than .as_list() (which I would still recommend doing)
This PR has the better tests.

[araujof]

I also had a fix for this in #921 - we can merge either PR, doesn't matter to me. For comparison, I had gone with using context.view_for_generation() rather than .as_list() (which I would still recommend doing) This PR has the better tests.

Thanks @psschwei . I wasn't sure what method to use, and decided to go with as_list as it would show non empty views for stateless SimpleContext.

backend = _MockBackend()
ctx = SimpleContext.from_previous(SimpleContext(), CBlock("prior turn"))
action = Instruction("Context view test")
await aact(action, ctx, backend, strategy=None)

If the correct behavior is to return an empty view in this case, we could change it to use .view_for_generation(). In this case, we would need to update unit test test_component_pre_execute_payload_has_context_view to reflect that SimpleContext.view_for_generation() always returns [] (by design, it's stateless, no history sent to the model).

@jakelorocco any thoughts? Happy to merge #921 into this PR if view_for_generation() is right approach here.

I could be wrong, but if we are using view_for_generation, couldn't one just use the generation hook? Maybe, at the component hook, one would want to have the complete context? I defer to the Mellea experts on this one :)

[psschwei]

My thought with using view_for_generation was just to mirror what the model sees, but I don't have super strong opinions here and will defer to you and @jakelorocco

[planetf1]

no notable issues noted - defer to jake on approval

[jakelorocco]

I think I would prefer context.view_for_generation. My reasoning is that that view is what the generation will see (same as Paul's logic).

If we need additional information to do something in the hook, I think we should modify the hook to pass along the full context and let the user call the necessary functions.