mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-13 03:13:41 +08:00
fix(context): never compact across a split tool_call ↔ tool_response pair
This commit is contained in:
matevip
parent
ae3d27922d
commit
37894a6978
@ -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<Message> oldMessages = new ArrayList<>(messages.subList(headEnd, tailStart));
|
||||
List<Message> 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.
|
||||
*
|
||||
* <p>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.
|
||||
*
|
||||
* <p>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.
|
||||
*
|
||||
* <p>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<Message> 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<String, Integer> assistantIdxById = new java.util.HashMap<>();
|
||||
// Map: tool_call_id -> max response index closing it.
|
||||
java.util.Map<String, Integer> 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。
|
||||
*/
|
||||
|
||||
@ -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.
|
||||
*
|
||||
* <p>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;
|
||||
}
|
||||
|
||||
@ -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}.
|
||||
*
|
||||
* <p>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.
|
||||
*
|
||||
* <p>Conventions used by these tests:
|
||||
* <ul>
|
||||
* <li>{@code asst(id1, id2, ...)} — assistant message carrying tool_calls</li>
|
||||
* <li>{@code resp(id, ...)} — tool response message closing the listed ids</li>
|
||||
* <li>"split" means the candidate cut falls between an assistant and one
|
||||
* of its responses; the algorithm must move the cut backward until no
|
||||
* split remains, or signal skip-compaction by returning {@code headEnd}.</li>
|
||||
* </ul>
|
||||
*/
|
||||
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<Message> 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<Message> 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<Message> 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<Message> 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<Message> 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<Message> 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<Message> 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<Message> 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<Message> 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<AssistantMessage.ToolCall> 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();
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user