mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-13 03:13:41 +08:00
fix(executor): spill raw tool result before falling back to inline truncate
This commit is contained in:
matevip
parent
2ef806aa64
commit
8f49b0ec92
@ -79,22 +79,70 @@ public class ToolExecutionExecutor {
|
||||
);
|
||||
|
||||
/**
|
||||
* Layer 1 — hard truncation cap applied to every tool result before it
|
||||
* reaches ToolResultStorage (Layer 2 spill) or the LLM prompt.
|
||||
* Inline hard-truncate cap for a single tool result. Acts as the fallback
|
||||
* when raw-first spill cannot run (storage disabled, tool excluded, body
|
||||
* already at-or-below the spill threshold, or disk write failed).
|
||||
*
|
||||
* <p>Two-level budget chain (RFC-008 / RFC-06 D-5):
|
||||
* <p>Per-tool-result handling chain:
|
||||
* <pre>
|
||||
* raw tool result
|
||||
* → truncateToolResult(..., MAX_TOOL_RESULT_CHARS=8000) // Layer 1: hard cap
|
||||
* → persistIfOversized(..., perResultThresholdChars=16000) // Layer 2: spill to disk
|
||||
* → enforceTurnBudget(..., perTurnBudgetChars=32000) // Layer 3: per-turn aggregate
|
||||
* raw tool result (full bytes)
|
||||
* → spillRawOrTruncate(...)
|
||||
* ├─ persistIfOversized(...) tries to write the raw body to disk
|
||||
* │ when size > perResultThresholdChars and tool is not
|
||||
* │ in the spill exclusion list. Returns a SPILL_MARKER preview
|
||||
* │ on success, or the original string otherwise.
|
||||
* └─ if no SPILL_MARKER on the return, truncateToolResult(...)
|
||||
* caps inline to MAX_TOOL_RESULT_CHARS so a multi-MB raw
|
||||
* body never enters the model prompt.
|
||||
* → enforceTurnBudget(..., perTurnBudgetChars=32000) // per-turn aggregate
|
||||
* </pre>
|
||||
* Layer 1 runs first and is intentionally kept at 8000 to prevent oversized
|
||||
* results from inflating the prompt. Layers 2/3 thresholds are configured in
|
||||
* {@link ToolResultProperties} and application.yml.
|
||||
* Spill must see the RAW result so the full output is preserved on disk
|
||||
* and the model can call {@code read_file} on the spill path. Truncating
|
||||
* before spilling would write a pre-shortened blob to disk, defeating the
|
||||
* "ground truth on disk" guarantee. {@link ToolResultProperties} controls
|
||||
* the thresholds; this constant stays in code because it is the safety
|
||||
* net for the failure case and should not vary by deployment.
|
||||
*/
|
||||
private static final int MAX_TOOL_RESULT_CHARS = 8000;
|
||||
|
||||
/**
|
||||
* Raw-first spill: try to write the full result to disk via the spill
|
||||
* store; only fall back to inline hard-truncate when no spill marker
|
||||
* comes back. Caller distinguishes spill success from "returned
|
||||
* unchanged" by checking {@link ToolResultStorage#SPILL_MARKER_PREFIX}
|
||||
* on the returned string — otherwise an IO failure or under-threshold
|
||||
* body would slip through indistinguishable from a successful spill,
|
||||
* and a multi-MB raw body could end up in the model prompt.
|
||||
*
|
||||
* <p>Package-private + static so the spill/truncate decision is unit
|
||||
* testable in isolation from the rest of the executor.
|
||||
*
|
||||
* @param storage spill store; {@code null} skips the spill attempt
|
||||
* @param maxTruncateChars fallback inline hard cap
|
||||
* @param result raw tool output (may be {@code null})
|
||||
* @param toolName used in the spill preview header
|
||||
* @param toolUseId unique within the conversation; becomes the file name
|
||||
* @param conversationId spill files are scoped per conversation; blank/null falls back to "unknown"
|
||||
* @param workspaceBasePath where the spill directory lives when set
|
||||
* @return the SPILL_MARKER preview when spill succeeded, otherwise the
|
||||
* original string (when ≤ threshold) or the inline-truncated string.
|
||||
*/
|
||||
static String spillRawOrTruncate(ToolResultStorage storage, int maxTruncateChars,
|
||||
String result, String toolName, String toolUseId,
|
||||
String conversationId, String workspaceBasePath) {
|
||||
if (result == null) return null;
|
||||
if (storage != null) {
|
||||
String safeConv = conversationId != null && !conversationId.isEmpty()
|
||||
? conversationId : "unknown";
|
||||
String candidate = storage.persistIfOversized(
|
||||
result, toolName, toolUseId, safeConv, workspaceBasePath);
|
||||
if (candidate != null && candidate.startsWith(ToolResultStorage.SPILL_MARKER_PREFIX)) {
|
||||
return candidate;
|
||||
}
|
||||
}
|
||||
return truncateToolResult(result, maxTruncateChars);
|
||||
}
|
||||
|
||||
/** 尾部错误模式检测 */
|
||||
private static final java.util.regex.Pattern ERROR_TAIL_PATTERN = java.util.regex.Pattern.compile(
|
||||
"(?i)\\b(error|exception|traceback|failed|fatal|panic|stack.?trace|errno)\\b");
|
||||
@ -581,16 +629,13 @@ public class ToolExecutionExecutor {
|
||||
toolCall.id(), toolName, DIRECT_TOOL_PLACEHOLDER);
|
||||
}
|
||||
|
||||
// RFC-008 Layer 1 first, then Layer 2 — match the non-replay path
|
||||
// in executeSingleTool so behavior stays symmetric across approval
|
||||
// replays. The caller-supplied conversationId scopes spill files
|
||||
// into the same per-conversation directory layout.
|
||||
result = truncateToolResult(result, MAX_TOOL_RESULT_CHARS);
|
||||
if (resultStorage != null && result != null) {
|
||||
String spillConv = conversationId != null && !conversationId.isEmpty() ? conversationId : "unknown";
|
||||
result = resultStorage.persistIfOversized(
|
||||
result, toolName, toolCall.id(), spillConv, workspaceBasePath);
|
||||
}
|
||||
// Raw-first spill, inline truncate as fallback. Symmetric with the
|
||||
// non-replay path in executeSingleTool. The caller-supplied
|
||||
// conversationId scopes spill files into the per-conversation
|
||||
// directory layout. See spillRawOrTruncate javadoc for why the
|
||||
// order matters.
|
||||
result = spillRawOrTruncate(resultStorage, MAX_TOOL_RESULT_CHARS,
|
||||
result, toolName, toolCall.id(), conversationId, workspaceBasePath);
|
||||
log.info("[ToolExecutor] Pre-approved tool {} returned {} chars{}", toolName, rawLen,
|
||||
result != null && result.length() < rawLen ? " (now " + result.length() + " after spill/truncate)" : "");
|
||||
events.add(GraphEventPublisher.toolComplete(toolCall.id(), toolName, result, true));
|
||||
@ -806,20 +851,16 @@ public class ToolExecutionExecutor {
|
||||
}
|
||||
}
|
||||
|
||||
// RFC-008 Layer 1: hard truncation cap to prevent oversized results
|
||||
// from inflating the prompt. Runs FIRST (before spill) so the spill
|
||||
// store doesn't need to handle multi-MB writes for run-of-the-mill
|
||||
// greps that happen to spit out a long stdout.
|
||||
result = truncateToolResult(result, MAX_TOOL_RESULT_CHARS);
|
||||
// RFC-008 Layer 2: spill oversized results to disk and replace
|
||||
// with preview + path. Falls back to truncation when spilling is
|
||||
// disabled or fails. Spill preserves the full output (read_file can
|
||||
// retrieve it); the Layer 1 truncation above already capped the
|
||||
// inline portion, so this layer mostly catches near-cap residues.
|
||||
if (resultStorage != null && result != null) {
|
||||
result = resultStorage.persistIfOversized(
|
||||
result, toolName, pc.toolCall.id(), pc.conversationId, pc.workspaceBasePath);
|
||||
}
|
||||
// Raw-first spill: write the full output to disk and replace
|
||||
// with preview + path so the model can call read_file for the
|
||||
// ground truth. Fall back to inline truncate only when spilling
|
||||
// is disabled, the tool is on the exclusion list, the body is
|
||||
// already under the spill threshold, or the disk write fails.
|
||||
// Truncating before spilling would persist a pre-shortened body
|
||||
// to disk and silently lose data the model could otherwise
|
||||
// recover.
|
||||
result = spillRawOrTruncate(resultStorage, MAX_TOOL_RESULT_CHARS,
|
||||
result, toolName, pc.toolCall.id(), pc.conversationId, pc.workspaceBasePath);
|
||||
log.info("[ToolExecutor] Tool {} returned {} chars{}", toolName, rawLen,
|
||||
result != null && result.length() < rawLen ? " (now " + result.length() + " after spill/truncate)" : "");
|
||||
events.add(GraphEventPublisher.toolComplete(pc.toolCall.id(), toolName, result, true));
|
||||
|
||||
@ -40,12 +40,26 @@ public class ToolResultProperties {
|
||||
private boolean enabled = true;
|
||||
|
||||
/**
|
||||
* Layer 2 — a single tool result larger than this is spilled to disk.
|
||||
* The executor evaluates this against the raw result before applying the
|
||||
* final inline cap, so oversized content is preserved before it is shortened
|
||||
* for the model request.
|
||||
* Per-result spill threshold. A single tool result larger than this is
|
||||
* spilled to disk and the in-context view is replaced with a short
|
||||
* preview + path so the model can call {@code read_file} on demand.
|
||||
*
|
||||
* <p>Aligned with {@code ToolExecutionExecutor.MAX_TOOL_RESULT_CHARS}
|
||||
* (8000): the executor now tries to spill the RAW result first; only
|
||||
* when spilling is disabled, the tool is on {@link #excludedTools}, the
|
||||
* body is under this threshold, or the disk write fails, does it fall
|
||||
* back to truncating inline to 8000 chars. Keeping the threshold equal
|
||||
* to the truncate cap yields a single semantic ladder — above the
|
||||
* threshold means "preserved on disk", at-or-below means "stays inline
|
||||
* verbatim".
|
||||
*
|
||||
* <p>If you want to keep more text inline before spilling, raise this
|
||||
* value AND raise the executor's hard cap together; otherwise the
|
||||
* 8000-char fallback truncate would silently shorten anything between
|
||||
* this threshold and 8000 even when spill is disabled, defeating the
|
||||
* intent.
|
||||
*/
|
||||
private int perResultThresholdChars = 16000; // was 4000 — prevents WebSearch spill-to-disk
|
||||
private int perResultThresholdChars = 8000;
|
||||
|
||||
/**
|
||||
* Layer 3 — aggregate cap on combined response size in one tool turn.
|
||||
|
||||
@ -207,15 +207,17 @@ mate:
|
||||
per-category:
|
||||
shell: 120
|
||||
web: 30
|
||||
# RFC-008 Phase 3: tool-result three-layer budget (per-result spill + per-turn aggregate budget).
|
||||
# Layer 1 (per-tool cap) lives inside individual tools; Layer 2 spills oversized
|
||||
# single results to disk; Layer 3 enforces an aggregate cap on the combined
|
||||
# response size of one tool turn. The full output is preserved on disk and
|
||||
# the in-context preview points the agent at the spill file (read_file tool).
|
||||
# Tool-result budget (per-result spill + per-turn aggregate budget).
|
||||
# The executor tries to spill the RAW result first so the full output is
|
||||
# preserved on disk; the in-context preview points the agent at the spill
|
||||
# file via read_file. When spill is disabled, the tool is on the exclusion
|
||||
# list, the body is at or below the threshold, or the disk write fails,
|
||||
# the executor falls back to inline hard-truncation to the same character
|
||||
# cap. Per-turn aggregate caps the combined size across one tool turn.
|
||||
tool-result:
|
||||
enabled: true
|
||||
per-result-threshold-chars: 16000 # was 4000 — prevents WebSearch spill-to-disk
|
||||
per-turn-budget-chars: 32000 # was 16000 — headroom for multi-tool turns
|
||||
per-result-threshold-chars: 8000 # aligned with executor hard cap; > this size → spill, ≤ → inline verbatim
|
||||
per-turn-budget-chars: 32000 # headroom for multi-tool turns
|
||||
preview-head-chars: 800
|
||||
excluded-tool-inline-chars: 2500
|
||||
storage-base-dir: ""
|
||||
|
||||
@ -14,11 +14,14 @@ import java.util.concurrent.ExecutorService;
|
||||
import static org.junit.jupiter.api.Assertions.*;
|
||||
|
||||
/**
|
||||
* Tests for Lane D executor and config changes:
|
||||
* Tests for the tool-result executor pipeline and its associated config.
|
||||
*
|
||||
* <ul>
|
||||
* <li>D-4: ToolExecutionExecutor uses virtual thread executor</li>
|
||||
* <li>D-5: ToolResultProperties defaults updated to 16000/32000</li>
|
||||
* <li>Virtual-thread tool executor is wired with named carrier threads.</li>
|
||||
* <li>{@link ToolResultProperties} defaults stay aligned with the executor
|
||||
* inline hard cap so spill and truncate share one semantic threshold.</li>
|
||||
* <li>{@link ToolExecutionExecutor#spillRawOrTruncate} attempts spill on the
|
||||
* raw body first and falls back to truncation only when spill cannot run.</li>
|
||||
* </ul>
|
||||
*/
|
||||
class LaneDExecutorAndConfigTest {
|
||||
@ -68,19 +71,19 @@ class LaneDExecutorAndConfigTest {
|
||||
// ============================================================
|
||||
|
||||
@Nested
|
||||
@DisplayName("D-5: ToolResultProperties defaults updated")
|
||||
@DisplayName("ToolResultProperties defaults")
|
||||
class ToolResultPropertiesDefaultsTests {
|
||||
|
||||
@Test
|
||||
@DisplayName("perResultThresholdChars default is 16000 (was 4000)")
|
||||
@DisplayName("perResultThresholdChars default aligns with executor hard cap (8000)")
|
||||
void perResultThresholdCharsDefault() {
|
||||
ToolResultProperties props = new ToolResultProperties();
|
||||
assertEquals(16000, props.getPerResultThresholdChars(),
|
||||
"Default perResultThresholdChars should be 16000");
|
||||
assertEquals(8000, props.getPerResultThresholdChars(),
|
||||
"Default perResultThresholdChars should equal the executor's MAX_TOOL_RESULT_CHARS=8000");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("perTurnBudgetChars default is 32000 (was 16000)")
|
||||
@DisplayName("perTurnBudgetChars default is 32000")
|
||||
void perTurnBudgetCharsDefault() {
|
||||
ToolResultProperties props = new ToolResultProperties();
|
||||
assertEquals(32000, props.getPerTurnBudgetChars(),
|
||||
@ -101,15 +104,125 @@ class LaneDExecutorAndConfigTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Large results above 16000 still trigger spill (threshold boundary)")
|
||||
void thresholdBoundary() {
|
||||
@DisplayName("Per-result threshold matches the executor inline hard cap so spill and truncate share one ladder")
|
||||
void thresholdMatchesExecutorHardCap() throws Exception {
|
||||
ToolResultProperties props = new ToolResultProperties();
|
||||
// Results <= 16000 should NOT spill
|
||||
assertTrue(15000 <= props.getPerResultThresholdChars(),
|
||||
"A 15000-char result should be within threshold");
|
||||
// Results > 16000 should spill
|
||||
assertTrue(17000 > props.getPerResultThresholdChars(),
|
||||
"A 17000-char result should exceed threshold");
|
||||
Field field = ToolExecutionExecutor.class.getDeclaredField("MAX_TOOL_RESULT_CHARS");
|
||||
field.setAccessible(true);
|
||||
int hardCap = (int) field.get(null);
|
||||
assertEquals(hardCap, props.getPerResultThresholdChars(),
|
||||
"perResultThresholdChars must equal MAX_TOOL_RESULT_CHARS; misalignment would silently shorten "
|
||||
+ "bodies between the two values when spill is disabled.");
|
||||
}
|
||||
}
|
||||
|
||||
// ============================================================
|
||||
// Raw-first spill ordering — the critical issue #110 fix
|
||||
// ============================================================
|
||||
|
||||
@Nested
|
||||
@DisplayName("ToolExecutionExecutor.spillRawOrTruncate: raw body reaches disk before the inline cap")
|
||||
class SpillOrTruncateOrderingTests {
|
||||
|
||||
@TempDir
|
||||
Path tempDir;
|
||||
|
||||
private ToolResultStorage storage(int threshold, List<String> excluded) {
|
||||
ToolResultProperties props = new ToolResultProperties();
|
||||
props.setStorageBaseDir(tempDir.toString());
|
||||
props.setPerResultThresholdChars(threshold);
|
||||
props.setPreviewHeadChars(120);
|
||||
if (excluded != null) props.setExcludedTools(excluded);
|
||||
return new ToolResultStorage(props);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("raw body > threshold → spill writes full original bytes to disk and returns preview")
|
||||
void rawOverThresholdSpillsFullContent() throws Exception {
|
||||
ToolResultStorage st = storage(1000, null);
|
||||
String raw = "0123456789\n".repeat(2000); // ~22000 chars, well over both threshold AND hard cap
|
||||
int rawLen = raw.length();
|
||||
|
||||
String out = ToolExecutionExecutor.spillRawOrTruncate(
|
||||
st, 8000, raw, "web_search", "call-1", "conv-x", tempDir.toString());
|
||||
|
||||
assertTrue(out.startsWith(ToolResultStorage.SPILL_MARKER_PREFIX),
|
||||
"should return a spill preview when raw exceeds threshold");
|
||||
assertTrue(out.contains("full_chars=" + rawLen),
|
||||
"preview header must report the original size, proving the raw bytes were what we measured");
|
||||
|
||||
// The file on disk should be the FULL raw body — not the 8000-char truncate.
|
||||
// Path is encoded inside the preview as "path=/abs/path".
|
||||
java.util.regex.Matcher m = java.util.regex.Pattern.compile("path=(\\S+)").matcher(out);
|
||||
assertTrue(m.find(), "preview must include path=...");
|
||||
Path spillFile = Path.of(m.group(1));
|
||||
assertTrue(java.nio.file.Files.exists(spillFile), "spill file should have been created");
|
||||
String fileContent = java.nio.file.Files.readString(spillFile);
|
||||
assertEquals(rawLen, fileContent.length(),
|
||||
"spill file must contain the full raw body, not a pre-truncated copy");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("raw body > threshold but tool is on exclusion list → no spill, inline hard cap")
|
||||
void rawOverThresholdExcludedToolTruncatesOnly() {
|
||||
ToolResultStorage st = storage(1000, List.of("read_file"));
|
||||
String raw = "x".repeat(20000);
|
||||
|
||||
String out = ToolExecutionExecutor.spillRawOrTruncate(
|
||||
st, 8000, raw, "read_file", "call-1", "conv-x", tempDir.toString());
|
||||
|
||||
assertFalse(out.startsWith(ToolResultStorage.SPILL_MARKER_PREFIX),
|
||||
"excluded tool must not be spilled");
|
||||
assertTrue(out.length() <= 8000,
|
||||
"excluded body still must fit the inline hard cap (was " + out.length() + ")");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("raw body ≤ threshold → returned unchanged, no spill, no truncation marker added")
|
||||
void rawUnderThresholdInlineVerbatim() {
|
||||
ToolResultStorage st = storage(1000, null);
|
||||
String raw = "small body";
|
||||
|
||||
String out = ToolExecutionExecutor.spillRawOrTruncate(
|
||||
st, 8000, raw, "web_search", "call-1", "conv-x", tempDir.toString());
|
||||
|
||||
assertEquals(raw, out, "small bodies should pass through untouched");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("storage null → falls back to inline hard cap, never crashes")
|
||||
void nullStorageFallsBackToTruncate() {
|
||||
String raw = "x".repeat(20000);
|
||||
|
||||
String out = ToolExecutionExecutor.spillRawOrTruncate(
|
||||
null, 8000, raw, "web_search", "call-1", "conv-x", tempDir.toString());
|
||||
|
||||
assertFalse(out.startsWith(ToolResultStorage.SPILL_MARKER_PREFIX));
|
||||
assertTrue(out.length() <= 8000,
|
||||
"with no storage, body must still fit the inline hard cap");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("null result stays null (no NPE)")
|
||||
void nullResultStaysNull() {
|
||||
ToolResultStorage st = storage(1000, null);
|
||||
String out = ToolExecutionExecutor.spillRawOrTruncate(
|
||||
st, 8000, null, "web_search", "call-1", "conv-x", tempDir.toString());
|
||||
assertNull(out);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("blank conversationId is replaced with a safe 'unknown' bucket so spill still lands on disk")
|
||||
void blankConversationIdRoutesToUnknownBucket() {
|
||||
ToolResultStorage st = storage(1000, null);
|
||||
String raw = "y".repeat(20000);
|
||||
|
||||
String out = ToolExecutionExecutor.spillRawOrTruncate(
|
||||
st, 8000, raw, "web_search", "call-1", "", tempDir.toString());
|
||||
|
||||
assertTrue(out.startsWith(ToolResultStorage.SPILL_MARKER_PREFIX),
|
||||
"blank conversationId must not stop spill — caller can be the legacy executePreApproved path");
|
||||
assertTrue(out.contains("unknown"), "spill path should land under the 'unknown' bucket");
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user