bytechefhq / bytechefhq/bytechef
[feature] Conditionally place chat-memory advisor inside the tool loop when the memory type supports persisting tool messages
- Dominant language
- Java
- Stars
- 1k
- Forks
- 170
- Avg merge
- 11h 25m
- Merged PRs (30d)
- 115
Description
## Summary
Conditionally place the chat‑memory advisor **inside** the `ToolCallingAdvisor` tool loop when — and only when — the selected chat‑memory type supports persisting tool messages (the full tool request/response transcript). For memory types that don't, keep the current **outside‑the‑loop** placement.
Reference: Spring AI blog — [Composable Tool Calling → "Memory and the tool loop"](https://spring.io/blog/2026/06/15/spring-ai-composable-tool-calling#memory-and-the-tool-loop).
## Background
Since the migration to composable tool calling (Spring AI 2.0.0), we build the `ToolCallingAdvisor` manually in `AbstractAiAgentChatAction.getAdvisors(...)` so we can wrap the manager in `SuspendableToolCallingManager` for suspend/resume:
`server/libs/modules/components/ai/agent/src/main/java/com/bytechef/component/ai/agent/action/AbstractAiAgentChatAction.java:453`
```java
advisors.add(
ToolCallingAdvisor.builder()
.toolCallingManager(
new SuspendableToolCallingManager(toolCallingManager, (ActionContextAware) context))
.build());
```
The chat‑memory advisor is added just before it, using each memory component's default order:
- `MessageChatMemoryAdvisor` default order = `-2147483448` (`DEFAULT_CHAT_MEMORY_PRECEDENCE_ORDER`, `MIN+200`)
- `ToolCallingAdvisor` default order = `-2147483348` (`ToolCallingAdvisor.DEFAULT_ORDER`, `MIN+300`)
Because memory's order (`MIN+200`) is lower/higher‑precedence than the tool loop (`MIN+300`), the memory advisor always sits **outside** the loop today. That is correct and safe — but it means memory persists **only the final user/assistant exchange**; tool request/response messages are never written to the store. The model therefore cannot recall, on a later turn, which tools it called and what they returned.
## Current vs. desired behavior
**Current (outside the loop, always):**
- Loads history once before the loop; persists only final user + assistant messages.
- Matches Spring AI 1.x behavior; safe for all memory implementations.
- No `disableInternalConversationHistory()` needed (correct as‑is).
**Desired (conditional):**
- If the memory type declares it can persist tool messages, place its advisor **inside** the loop (order `> ToolCallingAdvisor.DEFAULT_ORDER`) so it captures the full tool transcript, giving the LLM richer cross‑turn context.
- When inside, the manually built `ToolCallingAdvisor` **must** call `.disableInternalConversationHistory()` to avoid double‑writing the transcript (both histories otherwise record the same intra‑turn messages). Note: the auto‑registered `ToolCallingAdvisor` handles this automatically, but we register it manually, so we must call it explicitly.
- Otherwise, keep the current outside‑the‑loop placement.
## Which memory types qualify
Per the blog, only some repositories persist tool messages safely inside the loop. Built‑in safe options: `InMemoryChatMemoryRepository`, `RedisChatMemoryRepository`, `Neo4jChatMemoryRepository`. JDBC‑backed memory needs the Spring‑AI‑Session community project.
Our chat‑memory components (`server/libs/modules/components/ai/agent/chat-memory/`):
| Component | Likely supports tool messages inside loop? |
|---|---|
| `chat-memory-in-memory` | Yes |
| `chat-memory-redis` | Yes |
| `chat-memory-neo4j` | Yes |
| `chat-memory-jdbc` / `chat-memory-builtin` | Needs verification (Spring‑AI‑Session) |
| `chat-memory-mongodb` / `chat-memory-cassandra` | Needs verification |
| `chat-memory-aws` | Needs verification |
| `chat-memory-vectorstore` | Separate advisor type (`VectorStoreChatMemoryAdvisor`) — verify semantics |
| `chat-memory-session` | Needs verification |
Each component owns its own advisor build site, so capability must be declared per component rather than centrally inferred.
## Proposed approach (sketch — open to design)
1. Add a capability flag to `ChatMemoryFunction.Result` (e.g. `boolean supportsToolMessagePersistence`) that each memory component sets. Default `false` to preserve today's behavior.
2. In `AbstractAiAgentChatAction.getAdvisors(...)`:
- If the flag is `true`, add the memory advisor with an order **greater than** the tool advisor's order (i.e. `> ToolCallingAdvisor.DEFAULT_ORDER`, or `> advisorOrder` if we ever override it), and call `.disableInternalConversationHistory()` on the `ToolCallingAdvisor` at line 453.
- Else keep the current placement/behavior unchanged.
3. Prefer adjusting the **memory advisor's** order over the tool advisor's order, so we don't disturb guardrail ordering (guardrails sit at `HIGHEST_PRECEDENCE` and `DEFAULT_CHAT_MEMORY_PRECEDENCE_ORDER - 1`). Note `MessageChatMemoryAdvisor` is immutable, so the order must be set at build time in each component (or the advisor rebuilt from `Result.chatMemory()` where the type allows).
## Risks / things to verify
- **Suspend/resume:** interaction with `SuspendableToolCallingManager` and `ConversationResume` — confirm that persisting intermediate tool messages doesn't conflict with the serialized `ConversationState` on resume.
- **Token/storage cost:** inside‑the‑loop persistence stores the full transcript → larger memory rows and more replayed tokens. Consider making it opt‑in per component only where it adds value.
- **`disableInternalConversationHistory()` pairing is mandatory** whenever a memory advisor goes inside the loop; missing it → duplicate writes.
## Acceptance criteria
- [ ] `ChatMemoryFunction.Result` (or equivalent) exposes whether the memory type persists tool messages.
- [ ] Memory components that support it (at least in‑memory/redis/neo4j) declare the capability; others remain `false`.
- [ ] `getAdvisors(...)` places supporting memory advisors inside the loop and calls `disableInternalConversationHistory()`; non‑supporting ones stay outside.
- [ ] No duplicate persistence for the inside‑the‑loop path (verified by test asserting a single write of tool messages).
- [ ] Existing outside‑the‑loop behavior is unchanged for non‑supporting memory types (regression test).
- [ ] Suspend/resume continues to work with an inside‑the‑loop memory type.
Contributor guide
Assessment
This issue has not been assessed yet.