From 419ee57cc884a693faae3086e47e1a30b0c69a67 Mon Sep 17 00:00:00 2001 From: matevip Date: Sat, 16 May 2026 14:51:14 +0800 Subject: [PATCH] feat(agent): structured compaction on prompt-too-long preserves prefix --- .../context/ConversationWindowManager.java | 138 ++++++++++- .../mate/agent/graph/node/ReasoningNode.java | 124 +++++++--- .../ConversationWindowManagerPtlTest.java | 230 ++++++++++++++++++ .../node/ReasoningNodePtlPromptTest.java | 150 ++++++++++++ 4 files changed, 604 insertions(+), 38 deletions(-) create mode 100644 mateclaw-server/src/test/java/vip/mate/agent/context/ConversationWindowManagerPtlTest.java create mode 100644 mateclaw-server/src/test/java/vip/mate/agent/graph/node/ReasoningNodePtlPromptTest.java 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 2f4b9f43..22482c7a 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 @@ -162,6 +162,21 @@ public class ConversationWindowManager { /** 每个会话的摘要冷却截止时间 */ private final ConcurrentHashMap summaryCooldownUntil = new ConcurrentHashMap<>(); + /** Per-conversation last-PTL-forced-compaction timestamp. The structured + * PTL retry path is guarded by {@link #PTL_FORCE_LLM_COOLDOWN_MS} — a + * second PTL hit within the cooldown falls straight back to tail-only + * trimming. Without this, a model that keeps regenerating tool-call + * loops can drive a chain of summary-LLM calls and lock the + * conversation in a compaction storm. */ + private final ConcurrentHashMap ptlForceCompactAt = new ConcurrentHashMap<>(); + + /** Cooldown window after a structured PTL compaction during which a + * follow-up PTL is downgraded to tail-only. Picked so a single ReAct + * loop that retries within seconds can't burn another summary LLM + * call, while still letting the next real conversation turn (minutes + * later) get a fresh structured pass. */ + private static final long PTL_FORCE_LLM_COOLDOWN_MS = 60_000L; + // ==================== 主入口 ==================== /** @@ -263,7 +278,7 @@ public class ConversationWindowManager { int tailTokenBudget = (int) (triggerThreshold * 0.20); return compactMessages(messages, historyBudget, tailTokenBudget, chatModel, - conversationId, agentId, totalTokens, spillsAtEntry); + conversationId, agentId, totalTokens, spillsAtEntry, "token_threshold"); } /** @@ -298,11 +313,12 @@ public class ConversationWindowManager { private List compactMessages(List messages, int historyBudget, int tailTokenBudget, ChatModel chatModel, String conversationId, Long agentId, - int preTokens, long spillsAtEntry) { + int preTokens, long spillsAtEntry, + String trigger) { broadcastCompactStatus(conversationId, "start", Map.of( "preTokens", preTokens, "messagesIn", messages.size(), - "trigger", "token_threshold" + "trigger", trigger )); // 动态计算尾部保护边界（替代固定 preserveRecentPairs） @@ -457,7 +473,7 @@ public class ConversationWindowManager { ? Math.max(0L, toolResultStorage.getSpillCount() - spillsAtEntry) : 0L; Map boundaryMetadata = new java.util.LinkedHashMap<>(); - boundaryMetadata.put("trigger", "token_threshold"); + boundaryMetadata.put("trigger", trigger); boundaryMetadata.put("preTokens", preTokens); boundaryMetadata.put("postTokens", resultTokens); boundaryMetadata.put("messagesSummarized", oldMessages.size()); @@ -1114,6 +1130,118 @@ public class ConversationWindowManager { // ==================== PTL 紧急压缩 ==================== + /** + * Structured PTL (Prompt Too Long) recovery — reuses the full + * {@link #compactMessages} pipeline (pair-safe boundary, soft/hard + * trim, MemoryProvider hook, LLM summary, anchor of the first user + * goal) under a forced-tight history budget so the retry actually fits. + *

+ * Differences vs the {@link #compactForRetry(List)} fallback: + *

    + *
  • Preserves the original user goal via anchor instead of dropping + * it with the head — long tasks lose context every PTL otherwise.
    • + *
  • Pair-safe cuts, so the retry doesn't break a + * {@code AssistantMessage.tool_calls} / {@code ToolResponseMessage} + * cluster and 400 the provider a second time.
    • + *
  • Runs through summary generation so semantic continuity (user + * preferences, completed steps) survives the trim.
    • + *
  • Tags the persisted boundary row with + * {@code trigger=prompt_too_long} so the summary is retrievable + * via the same {@code mate_conversation_summary} schema as a + * normal token-threshold compaction.
    • + *
+ *

+ * A 60s cooldown ({@link #PTL_FORCE_LLM_COOLDOWN_MS}) downgrades the + * second-and-subsequent PTL hit on one conversation to tail-only, so + * a model stuck in a tool-call retry loop can't drag the summary LLM + * along with it. + * + * @param messages Current history that overflowed the model window. + * @param chatModel Used for the summary generation step. + * @param conversationId Cooldown / cache key. + * @param agentId Drives the {@code MemoryProvider.onPreCompress} + * hook. Nullable — the hook is a no-op when null. + * @return Compacted history with summary + anchor + tail, or the + * {@link #compactForRetry(List)} tail-only fallback when the + * cooldown is active or the structured pass produces no + * reduction. {@code null} when the input is too small to + * compact (matches the legacy contract). + */ + public List compactForRetry(List messages, + ChatModel chatModel, + String conversationId, + Long agentId) { + if (messages == null || messages.size() <= 2) { + return null; + } + + // Sweep the cooldown map on every PTL entry. The summaryCache sweep + // already covers normal-compaction traffic via fitToWindow; without + // this call here, a conversation that only ever hits PTL never + // releases its ptlForceCompactAt entry. + evictExpiredEntries(); + + // Race-safe claim: compute is atomic per key, so two concurrent + // PTL hits on the same conv can't both pass the cooldown check. + // The {@code claimed} flag is set inside the atomic block so we can + // distinguish "this call's stamp won" from "previous call's stamp + // happened to equal our now" (Windows clock has 15 ms granularity — + // identity-on-timestamp would misfire for back-to-back invocations). + long now = System.currentTimeMillis(); + final boolean[] claimed = {false}; + ptlForceCompactAt.compute(conversationId, (k, prev) -> { + if (prev != null && now - prev < PTL_FORCE_LLM_COOLDOWN_MS) { + claimed[0] = false; + return prev; + } + claimed[0] = true; + return now; + }); + if (!claimed[0]) { + long prevStamp = ptlForceCompactAt.getOrDefault(conversationId, now); + long remainingMs = Math.max(0L, PTL_FORCE_LLM_COOLDOWN_MS - (now - prevStamp)); + log.warn("[ConversationWindow] PTL cooldown active for conv={} (remaining {} ms), falling back to tail-only", + conversationId, remainingMs); + broadcastCompactStatus(conversationId, "ptl_cooldown_skipped", Map.of( + "trigger", "prompt_too_long", + "cooldownRemainingMs", remainingMs)); + return compactForRetry(messages); + } + + int currentTokens = TokenEstimator.estimateTokens(messages); + // Force the history budget into the bottom quartile of current size + // — but never under 2k so the post-trim window still has room for + // summary + anchor + a couple of recent turns. Tail budget is one + // quarter of that so the recent window doesn't dominate. + int forcedBudget = Math.max(2000, currentTokens / 4); + int forcedTailBudget = forcedBudget / 4; + + log.warn("[ConversationWindow] PTL forced compaction: messages={}, currentTokens={}, forcedBudget={}, forcedTail={}", + messages.size(), currentTokens, forcedBudget, forcedTailBudget); + + // Note: no separate "ptl_start" broadcast — the inner compactMessages + // call broadcasts "start" with trigger="prompt_too_long" in its + // payload, which is sufficient differentiation for the frontend + // (one event per compaction, with the trigger field carrying the + // semantic distinction). + + // Spill count is the manager's private view of toolResultStorage — + // computed inside the manager so callers don't need to touch the + // storage SPI. + long spillsAtEntry = (toolResultStorage != null) ? toolResultStorage.getSpillCount() : 0L; + + List compacted = compactMessages(messages, forcedBudget, forcedTailBudget, + chatModel, conversationId, agentId, currentTokens, spillsAtEntry, + "prompt_too_long"); + + if (compacted == messages || TokenEstimator.estimateTokens(compacted) >= currentTokens) { + log.warn("[ConversationWindow] PTL structured compaction had no effect for conv={}, falling back to tail-only", + conversationId); + return compactForRetry(messages); + } + return compacted; + } + /** * PTL (Prompt Too Long) 恢复用的紧急压缩。 * 不调用 LLM 摘要，直接丢弃较旧消息，只保留最近 4 条。 @@ -1155,6 +1283,8 @@ public class ConversationWindowManager { private void evictExpiredEntries() { summaryCache.entrySet().removeIf(entry -> entry.getValue().isExpired(CACHE_TTL_MS)); + long ptlCutoff = System.currentTimeMillis() - PTL_FORCE_LLM_COOLDOWN_MS; + ptlForceCompactAt.entrySet().removeIf(entry -> entry.getValue() < ptlCutoff); } record CachedSummary(String summary, long createdAt) { diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/node/ReasoningNode.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/node/ReasoningNode.java index acfb4a1c..293a48ac 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/node/ReasoningNode.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/node/ReasoningNode.java @@ -237,17 +237,16 @@ public class ReasoningNode implements NodeAction { // ======= 构建 Prompt ======= String systemPrompt = accessor.systemPrompt(); - // RFC-049 follow-up: append a tool-use enforcement clause to every - // ReasoningNode call. Without this, models (especially DeepSeek thinking - // and Claude Opus) tend to "narrate" — emit a final_answer like "现在 - // 直接生成立项材料 docx" instead of actually calling renderDocx, which - // makes the graph silently terminate at final_answer_node with the - // narration as the user-facing reply. + // Append a tool-use enforcement clause to every ReasoningNode call. + // Without it, some models (notably DeepSeek thinking and Claude Opus) + // tend to "narrate" — emit a final_answer like "现在直接生成立项材料 + // docx" instead of actually calling renderDocx, which makes the + // graph silently terminate at final_answer_node with the narration + // as the user-facing reply. // - // Pattern adopted from hermes-agent's TOOL_USE_ENFORCEMENT_GUIDANCE - // (`/agent/prompt_builder.py:179-191`). Appended to systemPrompt rather - // than woven into the AgentEntity-stored prompt so it stays out of the - // user-editable agent UI but is still always-on at runtime. + // Appended at runtime rather than woven into the AgentEntity-stored + // prompt so it stays out of the user-editable agent UI but is still + // always-on for the runtime LLM. systemPrompt = systemPrompt + TOOL_USE_ENFORCEMENT; List messages = accessor.messages(); @@ -328,24 +327,16 @@ public class ReasoningNode implements NodeAction { } String workspaceBasePath = state.value(vip.mate.agent.graph.state.MateClawStateKeys.WORKSPACE_BASE_PATH, ""); - List promptMessages = new ArrayList<>(); - promptMessages.add(new SystemMessage(systemPrompt)); - promptMessages.add(new UserMessage(RuntimeContextInjector.buildContextMessage(workspaceBasePath))); + String agentIdStr = state.value(MateClawStateKeys.AGENT_ID, ""); + String userMsg = state.value(MateClawStateKeys.USER_MESSAGE, ""); - // Wiki 相关性注入：根据用户消息提取相关页面摘要 - if (wikiContextService != null) { - String agentIdStr = state.value(MateClawStateKeys.AGENT_ID, ""); - String userMsg = state.value(MateClawStateKeys.USER_MESSAGE, ""); - try { - Long parsedAgentId = Long.parseLong(agentIdStr); - String wikiRelevant = wikiContextService.buildRelevantContext(parsedAgentId, userMsg); - if (wikiRelevant != null && !wikiRelevant.isBlank()) { - promptMessages.add(new UserMessage(wikiRelevant)); - } - } catch (NumberFormatException ignored) { - // agentId 无法解析时跳过 wiki 注入 - } - } + // Build the non-history prefix ONCE. The PTL retry branch below + // reuses this list verbatim so the retried prompt has exactly the + // same system / runtime context / wiki injection as the original — + // the previous tail-only retry path silently dropped the wiki + // segment which led to "answer regressed after compaction" + // complaints on long sessions. + List nonHistoryPrefix = buildNonHistoryPrefix(systemPrompt, workspaceBasePath, agentIdStr, userMsg); if (conversationWindowManager != null) { // Pass conversationId + workspaceBasePath so oversized older @@ -355,6 +346,7 @@ public class ReasoningNode implements NodeAction { messages = conversationWindowManager.pruneOldToolResultsForModelInput( messages, conversationId, workspaceBasePath); } + List promptMessages = new ArrayList<>(nonHistoryPrefix); promptMessages.addAll(messages); // 请求级思考深度覆盖（ThinkingLevelHolder 由 AgentService 设置） @@ -397,19 +389,34 @@ public class ReasoningNode implements NodeAction { try { result = streamingHelper.streamCall(chatModel, prompt, conversationId, "reasoning"); - // PTL 处理：压缩后重试 + // PTL 处理：结构化压缩后重试。复用 nonHistoryPrefix 保证重试 + // Prompt 仍带 wiki / runtime context；早期的 tail-only 路径会把 + // wiki 段一起丢掉，重试后的 prompt 比原始更短少一层信息。 if (result.isPromptTooLong() && conversationWindowManager != null) { - log.warn("[ReasoningNode] Prompt too long, attempting compaction and retry"); - List compactedMessages = conversationWindowManager.compactForRetry(messages); + log.warn("[ReasoningNode] Prompt too long, attempting STRUCTURED compaction and retry"); + + // MateClawStateAccessor.agentId() returns String per state + // schema; the ConversationWindowManager hook expects Long + // (nullable — onPreCompress is a no-op when null). + Long agentIdLong = null; + if (!agentIdStr.isEmpty()) { + try { + agentIdLong = Long.parseLong(agentIdStr); + } catch (NumberFormatException ignored) { + // Same fallback as the non-history prefix builder above. + } + } + + List compactedMessages = conversationWindowManager.compactForRetry( + messages, chatModel, conversationId, agentIdLong); + if (compactedMessages != null && compactedMessages.size() < messages.size()) { - List retryPromptMessages = new ArrayList<>(); - retryPromptMessages.add(new SystemMessage(systemPrompt)); - retryPromptMessages.add(new UserMessage(RuntimeContextInjector.buildContextMessage(workspaceBasePath))); + // Reuse the SAME non-history prefix — wiki/runtime context preserved. + List retryPromptMessages = new ArrayList<>(nonHistoryPrefix); retryPromptMessages.addAll(compactedMessages); Prompt retryPrompt = new Prompt(retryPromptMessages, options); log.info("[ReasoningNode] Retrying with compacted messages: {} -> {} messages", messages.size(), compactedMessages.size()); - // compact retry 是第 2 次 LLM 调用，先递增再调用 nextLlmCallCount++; pushPhase(conversationId, "reasoning", Map.of( "iteration", accessor.iterationCount(), @@ -660,6 +667,55 @@ public class ReasoningNode implements NodeAction { return out; } + /** + * Build the part of the Prompt that does not depend on history messages: + * system prompt, workspace runtime context, and (when wiring permits) the + * wiki relevant-pages snippet. Extracted so the initial Prompt assembly + * and the PTL retry path can share one source of truth — historically + * these were two parallel code paths and the retry one silently dropped + * the wiki injection. + *

+ * {@code systemPrompt} is consumed as-is; the upstream callsite has + * already appended the tool-use enforcement clause, so this helper must + * NOT re-append it (doing so would duplicate the clause on every retry). + * + * @param systemPrompt Fully-built system prompt (with tool-use + * enforcement already appended upstream). + * @param workspaceBasePath Active workspace directory; passed to + * {@link RuntimeContextInjector}. + * @param agentIdStr Agent ID as carried in graph state — parsed + * to {@code Long} only when non-empty and + * numeric; otherwise the wiki segment is + * skipped (matches the pre-refactor behavior). + * @param userMsg Current user message used by + * {@code WikiContextService} to score + * relevance. + */ + // Package-private so ReasoningNodePtlPromptTest can directly assert on + // the wiki / runtime-context layout; the production callsites inside + // this class call it via {@code this.buildNonHistoryPrefix(...)} so + // narrowing the visibility doesn't change behavior. + List buildNonHistoryPrefix(String systemPrompt, + String workspaceBasePath, + String agentIdStr, + String userMsg) { + List prefix = new ArrayList<>(); + prefix.add(new SystemMessage(systemPrompt)); + prefix.add(new UserMessage(RuntimeContextInjector.buildContextMessage(workspaceBasePath))); + if (wikiContextService != null && agentIdStr != null && !agentIdStr.isEmpty()) { + try { + Long parsedAgentId = Long.parseLong(agentIdStr); + String wikiRelevant = wikiContextService.buildRelevantContext(parsedAgentId, userMsg); + if (wikiRelevant != null && !wikiRelevant.isBlank()) { + prefix.add(new UserMessage(wikiRelevant)); + } + } catch (NumberFormatException ignored) { + // agentId not numeric — skip wiki injection (matches prior behavior). + } + } + return prefix; + } + private void pushPhase(String conversationId, String phase, Map extra) { if (streamTracker == null || !StringUtils.hasText(conversationId)) { return; diff --git a/mateclaw-server/src/test/java/vip/mate/agent/context/ConversationWindowManagerPtlTest.java b/mateclaw-server/src/test/java/vip/mate/agent/context/ConversationWindowManagerPtlTest.java new file mode 100644 index 00000000..bc7dae2a --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/agent/context/ConversationWindowManagerPtlTest.java @@ -0,0 +1,230 @@ +package vip.mate.agent.context; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +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 org.springframework.ai.chat.model.ChatModel; +import org.springframework.ai.chat.model.ChatResponse; +import org.springframework.ai.chat.model.Generation; +import org.springframework.ai.chat.prompt.Prompt; +import vip.mate.config.ConversationWindowProperties; +import vip.mate.workspace.conversation.ConversationService; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Contract for the structured PTL (Prompt Too Long) recovery path. + *

+ * Verifies: + *

    + *
  1. A PTL hit runs the full {@link ConversationWindowManager#compactMessages} + * pipeline (anchor + summary + tail) under a forced-tight budget, not + * the legacy tail-only drop.
    2. + *
  2. Tool-call clusters are kept intact across the cut so the retry + * prompt doesn't 400 the provider a second time.
    3. + *
  3. The persisted boundary row carries + * {@code metadata.trigger = "prompt_too_long"} so the summary is + * retrievable distinctly from a normal token-threshold compaction.
    4. + *
  4. A second PTL within the 60s cooldown falls back to tail-only and + * does NOT invoke the summary LLM (avoids compaction storms).
    5. + *
+ */ +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +class ConversationWindowManagerPtlTest { + + @Mock private ChatModel chatModel; + @Mock private ConversationService conversationService; + + private ConversationWindowProperties properties; + private ConversationWindowManager manager; + + @BeforeEach + void setUp() { + properties = new ConversationWindowProperties(); + properties.setFirstUserAnchorEnabled(true); + properties.setFirstUserAnchorMaxTokens(400); + // memoryManager is null — onPreCompress hook is a no-op. + manager = new ConversationWindowManager(properties, null, conversationService); + + // ChatModel always returns a short, deterministic summary so the + // pipeline can persist a boundary row and we can assert on its + // metadata. + when(chatModel.call(any(Prompt.class))).thenAnswer(inv -> + makeChatResponse("STRUCTURED_SUMMARY_FROM_LLM")); + when(conversationService.saveCompressionSummaryReturningId( + anyString(), anyString(), anyInt(), any())).thenReturn(42L); + } + + @Test + @DisplayName("PTL structured pass: returns summary + anchor + tail, no broken tool-call pair, trigger=prompt_too_long") + void structuredCompactionLandsAnchorSummaryAndTail() { + List history = buildHistoryWithToolPairs(); + int sizeBefore = history.size(); + + List compacted = manager.compactForRetry(history, chatModel, "conv-ptl-1", 1L); + + // ---- shape: not null, smaller than input ---- + assertThat(compacted).isNotNull(); + assertThat(compacted.size()).isLessThan(sizeBefore); + + // ---- contains the structured summary marker + anchor ---- + boolean hasStructuredSummary = compacted.stream() + .filter(m -> m instanceof UserMessage) + .map(Message::getText) + .anyMatch(t -> t != null + && t.startsWith(ConversationWindowManager.SUMMARY_PREFIX) + && t.contains("STRUCTURED_SUMMARY_FROM_LLM")); + assertThat(hasStructuredSummary) + .as("compacted history must include the LLM summary wrapped with SUMMARY_PREFIX") + .isTrue(); + boolean hasAnchor = compacted.stream() + .filter(m -> m instanceof UserMessage) + .map(Message::getText) + .anyMatch(t -> t != null && t.startsWith(ConversationWindowManager.ANCHOR_PREFIX)); + assertThat(hasAnchor) + .as("anchor of the original user goal must be present") + .isTrue(); + + // ---- pair safety: every AssistantMessage with tool_calls keeps its + // matching ToolResponseMessages adjacent ---- + assertNoBrokenToolPair(compacted); + + // ---- the boundary persistence path tags trigger = "prompt_too_long" ---- + @SuppressWarnings("unchecked") + ArgumentCaptor> metaCaptor = ArgumentCaptor.forClass(Map.class); + verify(conversationService).saveCompressionSummaryReturningId( + org.mockito.ArgumentMatchers.eq("conv-ptl-1"), + anyString(), anyInt(), metaCaptor.capture()); + assertThat(metaCaptor.getValue()) + .containsEntry("trigger", "prompt_too_long") + .containsKey("preTokens") + .containsKey("postTokens"); + } + + @Test + @DisplayName("PTL cooldown: second call within 60s falls back to tail-only, no extra ChatModel.call") + void secondPtlWithinCooldownFallsBackToTailOnly() { + List history = buildHistoryWithToolPairs(); + + // First call exercises the structured path → ChatModel.call invoked + // for summary generation. + manager.compactForRetry(history, chatModel, "conv-cooldown", 1L); + verify(chatModel, times(1)).call(any(Prompt.class)); + + // Second call within cooldown — tail-only fallback, no further LLM call. + List second = manager.compactForRetry(history, chatModel, "conv-cooldown", 1L); + assertThat(second).isNotNull(); + // Tail-only path drops summary + anchor — no SUMMARY_PREFIX in the + // second result (this is what differentiates it from the structured + // path even when both happen to return ≤ 4 messages). + assertThat(second.stream().anyMatch(m -> { + String t = m.getText(); + return t != null && t.startsWith(ConversationWindowManager.SUMMARY_PREFIX); + })).isFalse(); + // Critical: the summary LLM was NOT called a second time. + verify(chatModel, times(1)).call(any(Prompt.class)); + } + + @Test + @DisplayName("Tiny history (≤ 2 messages) returns null without touching ChatModel") + void tinyHistoryReturnsNull() { + List tiny = List.of(new UserMessage("hi"), new AssistantMessage("hello")); + List result = manager.compactForRetry(tiny, chatModel, "conv-tiny", 1L); + assertThat(result).isNull(); + verify(chatModel, never()).call(any(Prompt.class)); + } + + // ---------- helpers ---------- + + /** + * Build a 50-message history: alternating user/assistant turns plus five + * intact assistant.tool_calls → ToolResponseMessage clusters scattered + * through it. Each filler message carries ~300 chars so the total token + * count is comfortably large enough to push the structured budget into + * the "needs LLM summary" range. + */ + private static List buildHistoryWithToolPairs() { + List msgs = new ArrayList<>(); + String filler = "x".repeat(300); + msgs.add(new UserMessage("ORIGINAL_USER_GOAL: investigate the bug in module X")); + for (int i = 0; i < 22; i++) { + msgs.add(new AssistantMessage("assistant turn " + i + " " + filler)); + msgs.add(new UserMessage("user turn " + i + " " + filler)); + } + // Append five tool-call clusters at the tail half so the pair-safe cut + // has work to do (the cut may walk through them). + for (int i = 0; i < 5; i++) { + String callId = "call-" + i; + AssistantMessage call = AssistantMessage.builder() + .content("calling tool " + i) + .toolCalls(List.of(new AssistantMessage.ToolCall( + callId, "function", "search", "{\"q\":\"q" + i + "\"}"))) + .build(); + ToolResponseMessage response = ToolResponseMessage.builder() + .responses(List.of(new ToolResponseMessage.ToolResponse( + callId, "search", "result body " + i + " " + filler))) + .build(); + msgs.add(call); + msgs.add(response); + } + return msgs; + } + + /** + * Assert that every {@link AssistantMessage} carrying a non-empty + * {@code tool_calls} block in {@code messages} is immediately followed + * by at least one matching {@link ToolResponseMessage}, with one + * response per call id. Catches the "boundary split through a tool + * pair" failure mode the structured PTL path is specifically built to + * avoid. + */ + private static void assertNoBrokenToolPair(List messages) { + for (int i = 0; i < messages.size(); i++) { + if (messages.get(i) instanceof AssistantMessage am + && am.getToolCalls() != null && !am.getToolCalls().isEmpty()) { + // Every call id must appear in subsequent ToolResponseMessage(s) + // before any other AssistantMessage shows up. + java.util.Set outstanding = new java.util.LinkedHashSet<>(); + for (var c : am.getToolCalls()) outstanding.add(c.id()); + for (int j = i + 1; j < messages.size() && !outstanding.isEmpty(); j++) { + Message next = messages.get(j); + if (next instanceof ToolResponseMessage trm) { + for (var r : trm.getResponses()) outstanding.remove(r.id()); + } else if (next instanceof AssistantMessage) { + break; + } + } + assertThat(outstanding) + .as("AssistantMessage at index %d has unmatched tool_call ids", i) + .isEmpty(); + } + } + } + + private static ChatResponse makeChatResponse(String text) { + AssistantMessage am = new AssistantMessage(text); + return new ChatResponse(List.of(new Generation(am))); + } +} diff --git a/mateclaw-server/src/test/java/vip/mate/agent/graph/node/ReasoningNodePtlPromptTest.java b/mateclaw-server/src/test/java/vip/mate/agent/graph/node/ReasoningNodePtlPromptTest.java new file mode 100644 index 00000000..9e5570bf --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/agent/graph/node/ReasoningNodePtlPromptTest.java @@ -0,0 +1,150 @@ +package vip.mate.agent.graph.node; + +import org.junit.jupiter.api.Test; +import org.springframework.ai.chat.messages.Message; +import org.springframework.ai.chat.messages.SystemMessage; +import org.springframework.ai.chat.messages.UserMessage; +import vip.mate.agent.AgentToolSet; +import vip.mate.wiki.service.WikiContextService; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Regression guard: wiki / runtime-context content must survive a + * {@code prompt_too_long} retry. + *

+ * An earlier PTL retry path in {@code ReasoningNode} reassembled the prompt + * with only the system prompt + workspace runtime context — the wiki + * relevant snippet that the initial assembly injected was silently dropped, + * so the retried prompt asked the model the same question with strictly + * less context. {@code buildNonHistoryPrefix(...)} now centralises the + * three-layer prefix so the initial assembly and the retry path consume the + * same returned list. This test pins that contract: the prefix list + * contains all three layers and the wiki layer reflects what + * {@link WikiContextService#buildRelevantContext} returned. + */ +class ReasoningNodePtlPromptTest { + + private static final String WIKI_RELEVANT_TEXT = + "[Wiki Relevant Pages]\n- module-X.md (matches 'investigate'): ..."; + + @Test + void prefixIncludesSystemRuntimeAndWikiSegments() { + WikiContextService wikiContextService = mock(WikiContextService.class); + when(wikiContextService.buildRelevantContext(eq(42L), anyString())) + .thenReturn(WIKI_RELEVANT_TEXT); + + ReasoningNode node = newNode(wikiContextService); + + List prefix = node.buildNonHistoryPrefix( + "you are a helpful assistant", + "/workspace/active", + "42", + "investigate the bug in module X"); + + // Three layers: System, runtime-context UserMessage, wiki UserMessage. + assertThat(prefix).hasSize(3); + assertThat(prefix.get(0)).isInstanceOf(SystemMessage.class); + assertThat(prefix.get(0).getText()).contains("you are a helpful assistant"); + assertThat(prefix.get(1)).isInstanceOf(UserMessage.class); + assertThat(prefix.get(2)).isInstanceOf(UserMessage.class); + assertThat(prefix.get(2).getText()).isEqualTo(WIKI_RELEVANT_TEXT); + } + + @Test + void buildIsDeterministicAcrossCalls_soInitialAndRetryShareIdenticalLayout() { + // Critical regression invariant: both Prompt assemblies (initial and + // PTL retry) consume the SAME list reference, so the wiki segment + // can never diverge between them. Belt-and-suspenders, also verify + // that two independent calls with the same inputs produce + // structurally identical output. + WikiContextService wikiContextService = mock(WikiContextService.class); + when(wikiContextService.buildRelevantContext(eq(42L), anyString())) + .thenReturn(WIKI_RELEVANT_TEXT); + + ReasoningNode node = newNode(wikiContextService); + + List a = node.buildNonHistoryPrefix( + "sys", "/workspace", "42", "goal"); + List b = node.buildNonHistoryPrefix( + "sys", "/workspace", "42", "goal"); + + assertThat(a).hasSameSizeAs(b); + for (int i = 0; i < a.size(); i++) { + assertThat(a.get(i).getClass()).isEqualTo(b.get(i).getClass()); + assertThat(a.get(i).getText()).isEqualTo(b.get(i).getText()); + } + } + + @Test + void noWikiServiceWiredSkipsWikiSegment() { + // wikiContextService is optional — when null (e.g. minimal config or + // a test rig), the prefix should still be valid: just system + + // runtime context, no wiki layer. + ReasoningNode node = newNode(null); + + List prefix = node.buildNonHistoryPrefix( + "you are a helpful assistant", + "/workspace/active", + "42", + "investigate the bug in module X"); + + assertThat(prefix).hasSize(2); + assertThat(prefix.get(0)).isInstanceOf(SystemMessage.class); + assertThat(prefix.get(1)).isInstanceOf(UserMessage.class); + } + + @Test + void nonNumericAgentIdSkipsWikiSegment() { + WikiContextService wikiContextService = mock(WikiContextService.class); + ReasoningNode node = newNode(wikiContextService); + + List prefix = node.buildNonHistoryPrefix( + "sys", "/workspace", "not-a-number", "goal"); + + // Non-numeric agentId is the contract carried over from the + // pre-refactor codebase — skip wiki injection rather than throwing. + assertThat(prefix).hasSize(2); + verify(wikiContextService, + org.mockito.Mockito.never()).buildRelevantContext( + org.mockito.ArgumentMatchers.anyLong(), anyString()); + } + + @Test + void blankWikiResultSkipsWikiSegment() { + WikiContextService wikiContextService = mock(WikiContextService.class); + when(wikiContextService.buildRelevantContext(eq(42L), anyString())) + .thenReturn(" "); // blank → drop the layer + + ReasoningNode node = newNode(wikiContextService); + + List prefix = node.buildNonHistoryPrefix( + "sys", "/workspace", "42", "goal"); + + assertThat(prefix).hasSize(2); + } + + private static ReasoningNode newNode(WikiContextService wikiContextService) { + // 9-arg constructor — explicit supportsReasoningEffort + empty + // tool set, nulls for the streaming / conversation-window deps we + // don't exercise here. + AgentToolSet emptyTools = AgentToolSet.fromCallbacks(List.of(), List.of()); + return new ReasoningNode( + /* chatModel */ null, + /* toolSet */ emptyTools, + /* reasoningEffort */ null, + /* supportsReasoningEffort */ false, + /* streamingHelper */ null, + /* conversationWindowManager */ null, + /* streamTracker */ null, + /* maxOutputTokens */ 1024, + wikiContextService); + } +}