diff --git a/mateclaw-server/src/main/java/vip/mate/agent/context/ConversationWindowManager.java b/mateclaw-server/src/main/java/vip/mate/agent/context/ConversationWindowManager.java index 8559cf5b..09e6d19f 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/context/ConversationWindowManager.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/context/ConversationWindowManager.java @@ -263,6 +263,19 @@ public class ConversationWindowManager { return messages; } + // Pair safety: never split an AssistantMessage's tool_calls from its + // matching ToolResponseMessages. The cut may walk forward (i.e. the + // tail grows) until every call/response cluster lives on one side of + // the boundary. If no safe cut survives the walk, skip compaction — + // a broken pair would 400 every OpenAI-compatible provider, which is + // strictly worse than letting context cross the budget by one extra + // turn. + int pairSafeCut = enforcePairSafeBoundary(messages, headEnd, tailStart); + if (pairSafeCut <= headEnd) { + return messages; + } + tailStart = pairSafeCut; + List oldMessages = new ArrayList<>(messages.subList(headEnd, tailStart)); List recentMessages = messages.subList(tailStart, messages.size()); @@ -406,6 +419,113 @@ public class ConversationWindowManager { return Math.max(cutIdx, headEnd + 1); } + /** + * Adjust the candidate boundary so an {@link AssistantMessage}'s + * {@code toolCalls} are never separated from their matching + * {@link ToolResponseMessage}s. + * + *

Walks forward, collecting every {@code tool_call_id}'s assistant + * index and the indices of its matching responses. Whenever an + * assistant in the prefix has at least one response in the tail, the + * cut moves backward to that assistant — pulling the whole cluster + * into the tail. The walk repeats until convergence because moving + * the cut can expose pairs that were previously fully in the tail. + * + *

The method preserves pair integrity above any other concern. If + * the cut collapses all the way to {@code headEnd}, callers must + * interpret the return as "skip compaction this turn" — splitting a + * pair would produce HTTP 400 on every OpenAI-compatible provider, + * which is a worse failure mode than letting context grow by one turn. + * + *

An orphan {@code ToolResponseMessage} (id matching no + * assistant in scope) does not trigger movement; the upstream code + * paths should never produce one, and logging at WARN gives us a + * breadcrumb if they ever do. + * + * @return adjusted cut index, or {@code headEnd} when no pair-safe + * cut larger than {@code headEnd} can be produced. + */ + // Package-private so unit tests in the same package can drive it directly + // without standing up a ChatModel + the rest of the compactMessages pipeline. + int enforcePairSafeBoundary(List messages, int headEnd, int tailStart) { + if (tailStart <= headEnd || tailStart >= messages.size()) { + return tailStart; + } + int cut = tailStart; + int safety = messages.size() + 1; // hard guard against pathological loops + while (safety-- > 0) { + // Map: tool_call_id -> earliest assistant index that issued it. + java.util.Map assistantIdxById = new java.util.HashMap<>(); + // Map: tool_call_id -> max response index closing it. + java.util.Map latestResponseIdxById = new java.util.HashMap<>(); + + for (int i = headEnd; i < messages.size(); i++) { + Message m = messages.get(i); + if (m instanceof AssistantMessage am && am.getToolCalls() != null) { + for (AssistantMessage.ToolCall tc : am.getToolCalls()) { + String tid = tc.id(); + if (tid == null || tid.isEmpty()) continue; + // Keep the first occurrence so the cut "snaps" to the + // earliest assistant for any duplicated ids; the same + // id should never repeat anyway. + assistantIdxById.putIfAbsent(tid, i); + } + } else if (m instanceof ToolResponseMessage trm) { + for (ToolResponseMessage.ToolResponse r : trm.getResponses()) { + String tid = r.id(); + if (tid == null || tid.isEmpty()) continue; + latestResponseIdxById.merge(tid, i, Math::max); + } + } + } + + // Find the earliest in-prefix assistant whose pair is split. + int earliestSplitAssistant = Integer.MAX_VALUE; + for (var e : assistantIdxById.entrySet()) { + String id = e.getKey(); + int aIdx = e.getValue(); + Integer rIdx = latestResponseIdxById.get(id); + if (rIdx == null) { + // Assistant issued a call but no response — orphan call, + // would already break the provider. Not a pair-split, ignore. + continue; + } + if (aIdx < cut && rIdx >= cut && aIdx < earliestSplitAssistant) { + earliestSplitAssistant = aIdx; + } + if (aIdx >= cut && rIdx < cut) { + log.warn("[ConversationWindow] Orphan tool response in prefix without preceding assistant in tail (id={}); leaving boundary alone", + id); + } + } + + if (earliestSplitAssistant == Integer.MAX_VALUE) { + break; // converged: no splits remain + } + cut = earliestSplitAssistant; + } + + if (cut <= headEnd) { + log.info("[ConversationWindow] Pair-safe boundary collapsed to {} for conv: skipping compaction this turn to avoid splitting a tool_call ↔ tool_response pair", + headEnd); + return headEnd; + } + + int prefixSize = cut - headEnd; + int minPrefix = Math.max(0, properties.getPairSafeMinPrefixToCompact()); + if (prefixSize < minPrefix) { + log.info("[ConversationWindow] Pair-safe boundary left {} prefix message(s) (< minPrefix={}); skipping compaction", + prefixSize, minPrefix); + return headEnd; + } + + if (cut != tailStart) { + log.info("[ConversationWindow] Pair-safe boundary moved {} -> {} to keep tool_call ↔ tool_response pairs intact", + tailStart, cut); + } + return cut; + } + /** * 计算摘要字数预算：被压缩内容 token 的 20%，不低于 500、不超过 3000。 */ diff --git a/mateclaw-server/src/main/java/vip/mate/config/ConversationWindowProperties.java b/mateclaw-server/src/main/java/vip/mate/config/ConversationWindowProperties.java index 32b52514..5df3913b 100644 --- a/mateclaw-server/src/main/java/vip/mate/config/ConversationWindowProperties.java +++ b/mateclaw-server/src/main/java/vip/mate/config/ConversationWindowProperties.java @@ -37,4 +37,17 @@ public class ConversationWindowProperties { /** 摘要 token 预算下限（字数） */ private int summaryBudgetFloor = 500; + + /** + * Minimum prefix size (in messages) the pair-safe boundary must leave + * before compaction is allowed to run. After enforcing tool-call/response + * pair integrity the boundary may collapse so far forward that only a + * handful of messages remain in the prefix — at that point the + * compaction cost (a structured-summary LLM call) outweighs any token + * savings, and we may as well skip this turn. + * + *

Default 2 means "at least two old messages worth condensing". + * Set to 0 to always attempt compaction whenever a pair-safe cut exists. + */ + private int pairSafeMinPrefixToCompact = 2; } diff --git a/mateclaw-server/src/test/java/vip/mate/agent/context/ConversationWindowManagerPairSafeBoundaryTest.java b/mateclaw-server/src/test/java/vip/mate/agent/context/ConversationWindowManagerPairSafeBoundaryTest.java new file mode 100644 index 00000000..65e3f127 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/agent/context/ConversationWindowManagerPairSafeBoundaryTest.java @@ -0,0 +1,237 @@ +package vip.mate.agent.context; + +import org.junit.jupiter.api.Test; +import org.springframework.ai.chat.messages.AssistantMessage; +import org.springframework.ai.chat.messages.Message; +import org.springframework.ai.chat.messages.ToolResponseMessage; +import org.springframework.ai.chat.messages.UserMessage; +import vip.mate.config.ConversationWindowProperties; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Pair-safe boundary enforcement for {@link ConversationWindowManager}. + * + *

The compactor must never produce a prompt where an + * {@link AssistantMessage} carrying {@code tool_calls} is separated from + * the {@link ToolResponseMessage}s that close those calls. Provider APIs + * 400 on the broken sequence, which is strictly worse than letting the + * context cross the budget by one extra turn. + * + *

Conventions used by these tests: + *

+ */ +class ConversationWindowManagerPairSafeBoundaryTest { + + @Test + void cleanCutBetweenTurnsIsUnchanged() { + ConversationWindowManager mgr = newManager(0); + + // [0] user, [1] assistant(call-1), [2] response(call-1), + // [3] user, [4] assistant(call-2), [5] response(call-2) + List messages = List.of( + new UserMessage("q1"), + asst("call-1"), + resp("call-1"), + new UserMessage("q2"), + asst("call-2"), + resp("call-2") + ); + + // tailStart=3 — cuts cleanly between two fully-closed turns. + int cut = mgr.enforcePairSafeBoundary(messages, 0, 3); + + assertEquals(3, cut, "cut between completed turns must not move"); + } + + @Test + void cutLandingOnResponseMovesBackToOwningAssistant() { + ConversationWindowManager mgr = newManager(0); + + // [0] user, [1] assistant(call-1), [2] response(call-1), [3] user, [4] assistant(call-2), [5] response(call-2) + List messages = List.of( + new UserMessage("q1"), + asst("call-1"), + resp("call-1"), + new UserMessage("q2"), + asst("call-2"), + resp("call-2") + ); + + // tailStart=2 — splits call-1 (assistant in prefix, response in tail). + int cut = mgr.enforcePairSafeBoundary(messages, 0, 2); + + assertEquals(1, cut, + "cut must move to the assistant that issued call-1 so the pair lands in the tail together"); + } + + @Test + void cutSplittingAssistantWithMultipleToolCallsMovesEntireGroup() { + ConversationWindowManager mgr = newManager(0); + + // One assistant with TWO tool_calls; responses arrive in two separate + // ToolResponseMessages. Cutting between the responses must drag the + // assistant + both response messages into the tail together. + List messages = List.of( + new UserMessage("q"), + asst("call-1", "call-2"), + resp("call-1"), + resp("call-2"), + new UserMessage("next") + ); + + int cut = mgr.enforcePairSafeBoundary(messages, 0, 3); // between the two responses + + assertEquals(1, cut, + "splitting a multi-call assistant must move cut to the assistant index"); + } + + @Test + void cutSplittingMultiResponseMessagesForOneAssistantMovesBack() { + ConversationWindowManager mgr = newManager(0); + + // assistant(call-1, call-2), single ToolResponseMessage closing both. + List messages = List.of( + new UserMessage("q"), + asst("call-1", "call-2"), + ToolResponseMessage.builder().responses(List.of( + new ToolResponseMessage.ToolResponse("call-1", "tool_a", "x"), + new ToolResponseMessage.ToolResponse("call-2", "tool_b", "y") + )).build(), + new UserMessage("next") + ); + + // cut=2 → response message is in tail, assistant in prefix → split. + int cut = mgr.enforcePairSafeBoundary(messages, 0, 2); + + assertEquals(1, cut); + } + + @Test + void chainedPairSplitsConvergeAfterMultiplePasses() { + ConversationWindowManager mgr = newManager(0); + + // Three consecutive call/response cycles. Cutting in the middle + // exposes a split, and moving the cut back exposes another. + List messages = List.of( + asst("call-1"), // 0 + resp("call-1"), // 1 + asst("call-2"), // 2 + resp("call-2"), // 3 + asst("call-3"), // 4 + resp("call-3") // 5 + ); + + // cut=3 splits call-2 (assistant at 2, response at 3) → first pass moves to 2. + // After moving to 2, no more splits (call-1 is fully in prefix, call-3 fully in tail). + int cut = mgr.enforcePairSafeBoundary(messages, 0, 3); + assertEquals(2, cut); + + // cut=5 splits call-3 → moves to 4. cut=4, still good (no split). Convergence. + cut = mgr.enforcePairSafeBoundary(messages, 0, 5); + assertEquals(4, cut); + } + + @Test + void collapseToHeadEndSignalsSkip() { + ConversationWindowManager mgr = newManager(0); + + // Single assistant + response pair. Cutting anywhere splits it, + // so the safe boundary lands at headEnd → caller should skip compaction. + List messages = List.of( + asst("call-1"), + resp("call-1") + ); + + int cut = mgr.enforcePairSafeBoundary(messages, 0, 1); + + assertEquals(0, cut, "single unsafe pair must collapse to headEnd to signal skip"); + } + + @Test + void minPrefixThresholdSkipsTinyCompactions() { + // minPrefix=3 — after pair safety, if prefix < 3 messages, skip. + ConversationWindowManager mgr = newManager(3); + + List messages = List.of( + new UserMessage("q1"), + asst("call-1"), + resp("call-1"), + new UserMessage("q2") + ); + + // cut=3 would compress messages[0..3] = 3 items, meeting min. + // cut=1 would compress just messages[0..1] = 1 item, below min → skip. + int cut1 = mgr.enforcePairSafeBoundary(messages, 0, 3); + assertEquals(3, cut1, "3-message prefix meets the minimum"); + + int cut2 = mgr.enforcePairSafeBoundary(messages, 0, 1); + assertEquals(0, cut2, "1-message prefix is below the configured minimum → skip compaction"); + } + + @Test + void orphanResponseInTailDoesNotMoveBoundary() { + ConversationWindowManager mgr = newManager(0); + + // call-orphan has no preceding assistant — pure data anomaly. Algorithm + // should not try to "fix" it by moving the cut; it just leaves the + // boundary where it was and logs a warn. + List messages = List.of( + new UserMessage("q1"), + asst("call-1"), + resp("call-1"), + new UserMessage("q2"), + resp("call-orphan") + ); + + int cut = mgr.enforcePairSafeBoundary(messages, 0, 3); + + assertEquals(3, cut, "orphan response must not pull the boundary"); + } + + @Test + void tailStartAtOrBeyondMessagesSizeIsUnchanged() { + ConversationWindowManager mgr = newManager(0); + + List messages = List.of( + new UserMessage("a"), + new UserMessage("b") + ); + + assertEquals(2, mgr.enforcePairSafeBoundary(messages, 0, 2), + "boundary at end of list passes through"); + assertTrue(mgr.enforcePairSafeBoundary(messages, 0, 5) >= 0, + "out-of-range boundary stays sane"); + } + + // ------------------------------------------------------------------ helpers + + private static ConversationWindowManager newManager(int minPrefix) { + ConversationWindowProperties props = new ConversationWindowProperties(); + props.setPairSafeMinPrefixToCompact(minPrefix); + return new ConversationWindowManager(props, null, null); + } + + private static AssistantMessage asst(String... callIds) { + java.util.List calls = new java.util.ArrayList<>(); + for (String id : callIds) { + calls.add(new AssistantMessage.ToolCall(id, "function", "tool_" + id, "{}")); + } + return AssistantMessage.builder().content("").toolCalls(calls).build(); + } + + private static ToolResponseMessage resp(String callId) { + return ToolResponseMessage.builder().responses(List.of( + new ToolResponseMessage.ToolResponse(callId, "tool_" + callId, "ok") + )).build(); + } +}