Skip to content

refactor(event_loop): route message appends through Agent._append_messages#3131

Merged
opieter-aws merged 1 commit into
strands-agents:mainfrom
opieter-aws:agent-tasks/2805-append-chokepoint
Jul 9, 2026
Merged

refactor(event_loop): route message appends through Agent._append_messages#3131
opieter-aws merged 1 commit into
strands-agents:mainfrom
opieter-aws:agent-tasks/2805-append-chokepoint

Conversation

@opieter-aws

@opieter-aws opieter-aws commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description

The Agent exposes a single append chokepoint, _append_messages, that assigns a durable tracking_id and fires MessageAddedEvent for every message added to history. Three sites in event_loop.py — the assistant model response, the tool-result message, and the cancelled-tool-result message — bypassed it and hand-replicated those steps inline. That duplication was a concern raised in #2836 .

This routes all three appends through agent._append_messages(...), matching the structured-output force path that already used it. The duplicated _ensure_tracking_id / append / hook-invocation blocks collapse to one call each, and the now-unused MessageAddedEvent and _ensure_tracking_id imports are dropped. No behavior change: the same tracking_id assignment and MessageAddedEvent firing happen as before, and the yield ToolResultMessageEvent(...) ordering after the tool-result appends is preserved (the append is fully awaited before the yield). As a side benefit, the previously-untested cancel branch now gets the chokepoint's guarantees.

This is a Python-only change. The TypeScript SDK already routes every append through Agent._appendMessage / _appendMessageAndFireHooks and mints trackingId at Message construction rather than via an "ensure" step, so it has no equivalent duplication to consolidate.

The test fixtures needed a matching update. The event-loop tests use a Mock agent whose _append_messages was an auto-child mock that skipped the real logic; those fixtures now bind the real Agent._append_messages so appends assign tracking ids and fire MessageAddedEvent exactly as production does. The structured-output tests, which mock _append_messages and don't inspect agent.messages, switch from assert_called_once() to asserting the force prompt appears among the appended messages, since the assistant-response site now flows through the same mock too.

Related Issues

Follow-up to #2836

Documentation PR

No documentation changes needed — internal refactor with no public API or behavior change.

Type of Change

Other: Internal refactor — no behavior or public API change.

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce new warnings.

  • I ran hatch run prepare

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
  • My change is focused and reasonably small; I have split unrelated work into separate PRs
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@github-actions github-actions Bot added size/s python Pull requests that update python code chore Maintenance tasks, dependency updates, CI changes, refactoring with no user-facing impact area-agent Related to the agent class or general agent questions strands-running labels Jul 8, 2026
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Assessment: Approve

Clean, well-scoped refactor. I verified Agent._append_messages performs exactly the _ensure_tracking_idappendMessageAddedEvent sequence the three inline sites duplicated, that append is fully awaited before each yield ToolResultMessageEvent (ordering preserved), and that the now-unused imports were correctly dropped. All affected event-loop tests pass locally.

Review notes
  • Behavior preservation: No functional change — tracking_id assignment and MessageAddedEvent firing are identical to before across all three sites.
  • Scope: Correctly limited to event_loop.py; the remaining _ensure_tracking_id call sites (bidi agent, summarization/agentic context) are genuinely distinct flows and rightly left alone.
  • Tests: Binding the real _append_messages onto the mock fixtures is a good move — the appends now assign tracking ids and fire hooks like production. The structured-output test switching from field-by-field asserts to a whole-message equality check is an improvement aligned with the testing guide.
  • Test coverage gap (non-blocking): The one inline comment — the cancel-before-tool-execution branch flagged as a "side benefit" still lacks a test that exercises it.

Nice consolidation that removes real duplication called out in #2836.

Comment thread strands-py/src/strands/event_loop/event_loop.py
@opieter-aws opieter-aws marked this pull request as ready for review July 8, 2026 21:43
@opieter-aws opieter-aws requested a review from a team as a code owner July 8, 2026 21:43
@opieter-aws opieter-aws requested a review from liramon2 July 8, 2026 21:43
@liramon2 liramon2 requested a review from mehtarac July 9, 2026 14:04
Comment thread strands-py/src/strands/event_loop/event_loop.py
@liramon2 liramon2 removed the request for review from mehtarac July 9, 2026 14:16
@opieter-aws opieter-aws merged commit 818a985 into strands-agents:main Jul 9, 2026
54 of 55 checks passed
@opieter-aws opieter-aws deleted the agent-tasks/2805-append-chokepoint branch July 9, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-agent Related to the agent class or general agent questions chore Maintenance tasks, dependency updates, CI changes, refactoring with no user-facing impact python Pull requests that update python code size/s

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants