Skip to content

Conversation

@aszenz
Copy link
Contributor

@aszenz aszenz commented Feb 5, 2026

Q A
Bug fix? no
New feature? yes
Docs? yes
Issues Fix #1402
License MIT

Still not sure if we should change the default or remove this option altogether and move it to compression feature in #1549

@carsonbot carsonbot added Agent Issues & PRs about the AI Agent component Feature New feature Status: Needs Review labels Feb 5, 2026
@aszenz aszenz force-pushed the change/preserve-tool-msgs-by-default branch from d8353aa to 10ba7f5 Compare February 5, 2026 23:14
@chr-hertel chr-hertel added the BC Break Breaking the Backwards Compatibility Promise label Feb 6, 2026
@chr-hertel
Copy link
Member

Let's keep it here, but should we rename it to excludeToolMessage: false?
we're not hiding them, they're gone - and feels more consistent to includeSources

@aszenz aszenz force-pushed the change/preserve-tool-msgs-by-default branch 2 times, most recently from c348a2c to b99ce14 Compare February 9, 2026 21:13
private readonly ToolResultConverter $resultConverter = new ToolResultConverter(),
private readonly ?EventDispatcherInterface $eventDispatcher = null,
private readonly bool $keepToolMessages = false,
private readonly bool $keepToolMessages = true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meant this as well:

Suggested change
private readonly bool $keepToolMessages = true,
private readonly bool $excludeToolMessage = false,

needs a invert of the logic below

@aszenz aszenz force-pushed the change/preserve-tool-msgs-by-default branch 2 times, most recently from 46dd60b to aab79a2 Compare February 9, 2026 21:25
@aszenz aszenz force-pushed the change/preserve-tool-msgs-by-default branch from aab79a2 to 2535f83 Compare February 9, 2026 21:25
@aszenz aszenz requested a review from chr-hertel February 9, 2026 21:26
Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @aszenz - quite a change when it comes to token consumption, but the right default - i hope :D

I think we should still add a note in the UPGRADE.md - good to be merged after that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Agent Issues & PRs about the AI Agent component BC Break Breaking the Backwards Compatibility Promise Feature New feature Status: Reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Agent] Keep tool messages by default in agent processor

3 participants