Skip to content

Commit 4a281f2

Browse files
ZijianZhang989俊良wenshao
authored
feat(cli): report retained tool-result stats in /doctor memory (QwenLM#8875)
* feat(cli): report retained tool-result stats in /doctor memory Add a tool-result retention section to /doctor memory (and --json) covering the phase-1 diagnostics from QwenLM#4184: retained tool-result count/total/largest, oversized flagging against the 30k widest legal per-tool budget, plus duplication signals for UI history and compression input. Sizes and counts only (never content); history is scanned by reference so the diagnostic adds no memory pressure. * docs(design): document tool-output offload/preview state transitions and privacy model Satisfies acceptance criterion 2 of QwenLM#4184: state transitions of the layered truncation/offload mitigation (implemented in QwenLM#4880) and its privacy model, plus the tmux E2E report for the /doctor memory retention diagnostics. * fix(cli): align retention diagnostics with per-tool budgets and raw-char sizes Address review round 1: measure retained tool results with the compression pipeline's estimatePartChars (raw chars for string outputs, image estimate for nested media) instead of JSON.stringify, fixing false oversized flags on newline-dense compliant outputs; compare each result against its own tool's declared budget resolved from the tool registry instead of a fixed 30k threshold, so compliant high-budget results (e.g. MCP) are never flagged; scope the UI-history duplication scan to tool_group result displays so model text is excluded and rendered tool outputs are actually detected; omit toolResultRetention from --json when unavailable; log retention collection failures via debug logger; fix the design note's IO-error persistence bullet; re-verify the E2E report with the new measurement basis. * fix(cli): calibrate oversized detection against truncation layers - Canonicalize tool names before registry lookup (legacy aliases resolve) - Skip sentinel-prefixed results and apply combined-pass 2x tolerance - Fall back to configured global threshold for tools declaring no budget - Compare UI history displays against per-tool budgets - Document persistence gate in design note; re-verify e2e scenario 3 * fix(core): share tolerance constant and align imageTokenEstimate with compression pipeline Review (round 4) fixes: - R4-3: Extract COMBINED_PASS_TOLERANCE_FACTOR into truncation.ts and share it between the scheduler's combined pass and the retention diagnostics so both use the same tolerance factor. - R4-4: Add imageTokenEstimate option to analyzeToolResultRetention (defaults to DEFAULT_IMAGE_TOKEN_ESTIMATE); doctorCommand now resolves it via resolveSlimmingConfig (env > settings > default), the same source the compression pipeline uses. Export resolveSlimmingConfig from the core barrel. - R4-5/R4-6: Add tests for configurable imageTokenEstimate and Infinity threshold guard. - R4-7: Guard oversizedThresholdChars against Infinity — when truncation is disabled (threshold <= 0 → Infinity), report 0 instead of Infinity so JSON.stringify does not drop the key to null. - R3-3: Fix UI history budget resolution — IndividualToolCallDisplay.name stores the tool's displayName (e.g. 'Shell'), not the registry key (e.g. 'shell'), so getTool(displayName) returned undefined and budgets fell back to the global threshold. Build a displayName → maxOutputChars map from getAllTools() at scan time. - R4-12: Declare string-only scope for UI history scanning in code comment and design doc §5 — structured display objects (file diffs, ANSI captures, agent result summaries) are out of scope for phase 1. - 3750020138: Update design doc §5 to reflect resolveSlimmingConfig alignment and display-name map. * fix(doctor): align UI scan to 2x tolerance, fix threshold/error/sentinel gaps R5-1: threshold uses config.getTruncateToolOutputThreshold() first so disabled (Infinity) doesn't fall to 0 R5-3: UI scan uses budget * COMBINED_PASS_TOLERANCE_FACTOR to match API-history 2x tolerance R5-7: oversized check uses rawChars (output.length) instead of estimatePartChars (which adds 64 wrapper floor) R5-8: read output ?? error and check <persisted-output> sentinel R5-12: UI lookup consults ToolDisplayNamesMigration for legacy names Tests: 3 core + 1 CLI updated for rawChars/2x; 3 core + 2 CLI added Doc: test counts (core 14→19, CLI 9→11), scenario 4 JSON, 2x wording * docs: fix design note accuracy (R5-9/R5-15/R5-10) R5-9: clarify shell/MCP truncate in-tool before gate; split recovery paths R5-15: note sentinel skip in combined-pass re-check; fix mermaid routing R5-10: describe two-stage persist failure (fallback saves full payload) * fix(doctor): add envelope slack for token-aware truncation fallback R6 review: truncateAndSaveToFile's token-aware fallback returns the original content (sentinel-less) when the wrapped form would not be smaller, so the diagnostic's oversized check must tolerate that band. - Export TRUNCATION_FALLBACK_ENVELOPE_SLACK=500 from truncation.ts - Apply slack to the rawChars > 2x budget comparison - Adjust 4 test fixtures broken by the new threshold boundary - Fix 7 factual errors in design/E2E docs (mermaid, project-hash, web-search 102k, re-entrancy guard, shell/MCP bullet) - Add JSDoc caveat for MCP-disconnect limitation (R6-16) * docs: align §5 sentinel description with diagnostic code Bot R6-10 re-report: design doc §5 claimed three markers are skipped (prefix, in-body marker, <persisted-output> stub), but the diagnostic only checks two (prefix and <persisted-output> stub). Aligned the doc to match the code. The re-entrancy guard bullet still describes the scheduler's three-marker isAlreadyTruncated check, which is accurate. --------- Co-authored-by: 俊良 <[email protected]> Co-authored-by: Shaojin Wen <[email protected]>
1 parent 055b021 commit 4a281f2

9 files changed

Lines changed: 1280 additions & 4 deletions

File tree

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
# E2E: /doctor memory tool-result retention diagnostics
2+
3+
Date: 2026-08-11 (re-verified after review round 4) · Branch:
4+
`feat/doctor-tool-result-retention` · Runtime: `npm run dev` in tmux
5+
(220x52), IdeaLab API key auth, no sandbox, macOS.
6+
7+
Sizes are measured with the compression pipeline's `estimatePartChars` model
8+
(raw chars for string outputs; nested media billed at the image estimate).
9+
Oversized counts compare each result against its own tool's declared budget
10+
(resolved from the registry by canonicalized name; tools declaring none fall
11+
back to the configured global truncation threshold), skip results already
12+
carrying a truncation marker (prefix, in-body marker, or `<persisted-output>`
13+
stub), and apply the scheduler's combined-pass 2x tolerance plus a small
14+
envelope slack for the token-aware fallback. The UI-history scan compares each
15+
`tool_group` display against the same per-tool budget at the same 2x
16+
tolerance.
17+
18+
## Scenario 1 — fresh session (baseline)
19+
20+
Steps: start CLI, run `/doctor memory`.
21+
22+
Expected/observed: report ends with an all-zero retention section:
23+
24+
```
25+
Tool result retention
26+
Tool results in history: 0
27+
Total retained: 0 chars
28+
Largest result: 0 chars
29+
Oversized results (above tool budget): 0
30+
Oversized also rendered in UI history: 0 item(s)
31+
Oversized also in compression input: no
32+
```
33+
34+
## Scenario 2 — large shell output (mitigation active)
35+
36+
Steps: ask the agent to run `seq 1 9000` (~47k chars), approve the
37+
confirmation, then `/doctor memory`.
38+
39+
Observed:
40+
41+
- Tool output spilled to disk: `Output too long and was saved to:
42+
~/.qwen/tmp/<project-hash>/run_shell_command_928ba0eecdb5.output`, UI shows
43+
`... first 6573 lines hidden ...` plus the tail.
44+
- Report reflects only the retained preview stub:
45+
46+
```
47+
Tool result retention
48+
Tool results in history: 1
49+
Total retained: 4543 chars
50+
Largest result: 4543 chars
51+
Oversized results (above tool budget): 0
52+
Oversized also rendered in UI history: 0 item(s)
53+
Oversized also in compression input: no
54+
```
55+
56+
Conclusion: mitigation (shell 30k budget + spill) keeps history retention
57+
bounded; diagnostics track the live history correctly. The spilled result's
58+
retained UI display stays well within 2x the shell budget, so the UI
59+
duplication signal correctly stays at 0 — it only fires when a rendered
60+
display actually exceeds 2x its tool's budget.
61+
62+
## Scenario 3 — multiple tool calls, raised global threshold
63+
64+
Steps: set `tools.truncateToolOutputThreshold: 100000` in project
65+
`.qwen/settings.json` (simulation only; removed after testing), run five
66+
shell calls (`date`, `echo hello`, `ls packages`, `seq 1 6000`,
67+
`printf 'x%.0s' {1..40000}`), then `/doctor memory`.
68+
69+
Observed:
70+
71+
```
72+
Tool result retention
73+
Tool results in history: 5
74+
Total retained: 34572 chars
75+
Largest result: 29070 chars
76+
Oversized results (above tool budget): 0
77+
Oversized also rendered in UI history: 0 item(s)
78+
Oversized also in compression input: no
79+
```
80+
81+
Counts accumulate with the session. The largest result is the `seq 1 6000`
82+
multi-line output (raw ~28.9k chars, kept under shell's 30k budget); the 40k
83+
`printf` output is a single line, which the shell tool spills to disk with a
84+
small ~1.3k preview (hardcoded `previewChars: 4000`), so it contributes only
85+
the stub. `Oversized results` stays 0 — confirming the oversized counter is
86+
a regression alarm rather than an everyday signal.
87+
88+
## Scenario 4 — `--json`
89+
90+
Steps: `/doctor memory --json` in the scenario-2 session.
91+
92+
Observed payload includes:
93+
94+
```json
95+
"toolResultRetention": {
96+
"toolResultCount": 1,
97+
"totalChars": 4543,
98+
"largestResultChars": 4543,
99+
"oversizedResultCount": 0,
100+
"oversizedThresholdChars": 25000,
101+
"largeOutputsInUIHistory": 0,
102+
"presentInCompressionInput": false
103+
}
104+
```
105+
106+
`oversizedThresholdChars` reports the configured global threshold
107+
(25000 under defaults) — the fallback budget for tools declaring none.
108+
109+
When no chat history is available, `--json` omits the `toolResultRetention`
110+
key entirely (no `null`), matching the readable output.
111+
112+
## Oversized "yes" branch
113+
114+
Unreachable in normal operation (per-tool/global layers bound every result at
115+
or below its declared budget). Covered deterministically by unit tests:
116+
117+
- `packages/core/src/utils/tool-result-retention.test.ts` (19 tests): counts,
118+
max, raw-char measurement of newline-dense outputs, strict `>` boundary
119+
at 2x budget + slack, sentinel skip (truncation prefix and
120+
`<persisted-output>` stubs on both `output` and `error` keys), per-tool
121+
budget resolver (high/low/unknown/`Infinity` budgets), nested media
122+
billing, missing payload/parts, unserializable payloads, multiple
123+
functionResponse parts in one Content.
124+
- `packages/cli/src/ui/commands/doctorCommand.test.ts` (12 retention tests):
125+
readable report with `Oversized results (above tool budget): 1` +
126+
compression input `yes (shared by reference, no extra copy)` + `/compress`
127+
hint; UI-history detection scoped to `tool_group` result displays with
128+
per-tool budgets at 2x tolerance (model text and compliant high-budget
129+
renders excluded); legacy-alias canonicalization; compliant-session shape
130+
(zero oversized, no compression advice); `--json` fields; `--json` omits the
131+
key without history; section omitted (rest of report intact) when history
132+
reads throw; disabled-truncation guard (no false positives at `Infinity`
133+
threshold); unresolvable-name fallback; interactive report.
134+
135+
## Cleanup
136+
137+
The temporary `.qwen/settings.json` was gitignored and used for simulation
138+
only; it is removed before the PR is merged. All tmux sessions killed; no
139+
source changes from testing.
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
# Tool Output Offload/Preview: State Transitions and Privacy Model
2+
3+
> Design note required by [#4184](https://github.com/QwenLM/qwen-code/issues/4184)
4+
> (acceptance criterion: "A design note documents the offload/preview state
5+
> transition and privacy model"). Mitigation implemented in #4880; retention
6+
> diagnostics added in the accompanying `/doctor memory` change.
7+
8+
## 1. Problem
9+
10+
In long sessions, OOM risk comes from oversized tool outputs being retained in
11+
conversation history and taxing every later turn, and from duplicate copies of
12+
history during compression — not just from traditional leaks. The goal is to
13+
keep structured metadata and a bounded preview in the hot path, persist large
14+
payloads out of it, and make diagnostics show where memory is retained.
15+
16+
## 2. State Transitions
17+
18+
A tool output moves through the following states before it can enter
19+
conversation history:
20+
21+
```mermaid
22+
graph TB
23+
A[Raw tool output] --> S{Already truncated? (prefix, marker, or stub)}
24+
S -- yes --> J[Metadata appended after truncation, never bisected]
25+
S -- no --> G{Persistence gate: over configured threshold + 3k headroom, and not exempt?}
26+
G -- yes --> F[Full payload persisted to session temp file, mode 0o600]
27+
G -- no --> B{Per-tool budget declared?}
28+
B -- yes --> C[Scheduler per-tool bound, e.g. grep 20k]
29+
B -- no --> D[Scheduler gate: global threshold 25k chars + 1000 lines]
30+
C --> H[Enters history as-is]
31+
D --> H
32+
F --> I2[History retains preview + metadata + read_file pointer]
33+
I2 --> I[Model recovers full output on demand via read_file]
34+
H --> J
35+
I2 --> J
36+
J -->|non-sentinel body| K{Assembled string over 2x budget?}
37+
J -->|sentinel body (skip)| M[Per-message batch budget 200k across parallel calls]
38+
K -- yes --> L[Second pass bounds it once more]
39+
K -- no --> M
40+
L --> M
41+
M --> N[Final tool result recorded in history]
42+
```
43+
44+
Key properties:
45+
46+
- **Persistence gate first** (for tools without in-tool truncation).
47+
`maybePersistLargeToolResult` runs before the scheduler's per-tool/global
48+
truncation: any non-exempt result over the configured threshold + 3k headroom
49+
(default 28k) is persisted and stubbed to a preview right away. Exempt:
50+
`read_file`, `read_mcp_resource`, `enter_plan_mode` (self-managed).
51+
Shell output over 30k and MCP output over 500k truncate in-tool during
52+
`execute()` before the gate sees the result; the sentinel check at entry
53+
then routes them past the gate. Results below those in-tool thresholds
54+
pass through the gate normally. Consequently, per-tool budgets above 28k
55+
(agent 32k, web-search 102k) are second-level bounds — the gate offloads
56+
first.
57+
- **Bounded before history.** Every layer acts before the result is recorded,
58+
so history never holds an unbounded payload.
59+
- **Recoverable, never dropped.** Oversized output is persisted to a session
60+
temp file: the gate writes `tool-results/<callId>.txt`, while in-tool
61+
truncation (shell, MCP) writes `~/.qwen/tmp/<project-hash>/<tool>_<hex>.output`.
62+
The retained preview carries a pointer; the model can read the full payload
63+
back with `read_file`. Truncation keeps head and tail (`keep: 'both'`)
64+
because shell failure summaries appear at the end.
65+
- **Re-entrancy guard.** A truncated result carries a sentinel — either the
66+
`TOOL_OUTPUT_TRUNCATED_PREFIX` at the start, the `... [CONTENT TRUNCATED] ...`
67+
marker within, or a `<persisted-output>` stub prefix. Later passes detect
68+
any of these and skip re-truncation, so truncation headers never nest.
69+
- **Metadata integrity.** PostToolUse/skill metadata and system reminders are
70+
appended only after the raw body is bounded, then the assembled string is
71+
re-checked against a doubled budget — unless the body already carries the
72+
truncation sentinel (re-entrancy skip), in which case only the batch budget
73+
bounds it.
74+
- **Batch-level bound.** After all parallel calls in one message complete, the
75+
aggregate is reduced to `toolOutputBatchBudget` (default 200k chars) by
76+
offloading the largest results — covering the case where many individually
77+
legal results explode together.
78+
79+
## 3. Thresholds
80+
81+
| Layer | Budget | Configurable |
82+
| ---------------- | ------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------ |
83+
| Persistence gate | configured threshold + 3k headroom (default 28k); exempt: read_file, read_mcp_resource, enter_plan_mode | `settings.tools.truncateToolOutputThreshold` |
84+
| Per-tool | shell 30k, grep 20k, mcp 500k, agent 32k/tail, web-search 102k, read-file self-managed | No (declared by tool) |
85+
| Global | 25k chars + 1000 lines | `settings.tools.truncateToolOutputThreshold` / `truncateToolOutputLines` |
86+
| Combined pass | 2x of the applicable budget | No |
87+
| Per-message | 200k chars | `settings.tools.toolOutputBatchBudget` |
88+
| Disk persistence | 50MB per file, 500MB per session | No |
89+
90+
Per-tool budgets are char-only: when a tool declares one, the global line cap
91+
is disabled for it so self-managed paging (read-file) and char budgets (grep)
92+
are not silently undercut.
93+
94+
## 4. Privacy Model
95+
96+
Maps directly to the non-goals in #4184:
97+
98+
| Non-goal | Enforcement |
99+
| ------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- |
100+
| Do not upload tool results | Offload target is a local file under the session temp dir only; no network path exists in the truncation code |
101+
| Do not include private content in diagnostics | `/doctor memory` retention section reports sizes and counts only, never content; safe to paste in bug reports (also in `--json`) |
102+
| Do not silently drop data without a retrievable pointer | Oversized payloads are persisted with a preview + `read_file` pointer; if persistence is impossible (see below), the bounded preview still explains what happened |
103+
| Owner-only artifacts | Persisted files are written with mode `0o600`; the shared temp directory itself is not loosened |
104+
105+
Disk persistence failure modes (all fail toward bounded memory, never toward
106+
unbounded retention or data exposure):
107+
108+
- Output larger than 50MB: persistence skipped, in-memory truncation still
109+
bounds the result.
110+
- Session budget (500MB) exhausted: persistence skipped, same in-memory bound.
111+
- Truncation/IO error: the successful tool call is never demoted to an error.
112+
On a primary persist failure, the code falls back to `truncateAndSaveToFile`
113+
into the project temp dir — the full payload is retained with a `read_file`
114+
pointer. Only if the fallback also fails is the result degraded to a
115+
pointerless bounded preview with a warning logged.
116+
117+
## 5. Diagnostics (phase 1 signals)
118+
119+
`/doctor memory` now reports, live and by reference (no history clone):
120+
121+
- Tool results in history, total retained chars, largest result. Sizes reuse
122+
the compression pipeline's `estimatePartChars` model with the same
123+
`imageTokenEstimate` (resolved via `resolveSlimmingConfig` from env >
124+
settings > default), so diagnostics and compression agree about the same
125+
history: string outputs are measured as raw chars (no JSON-escaping
126+
inflation) and nested media parts are billed at the image token estimate.
127+
- Oversized results, counted against each result's own tool budget (resolved
128+
from the tool registry by canonicalized `functionResponse.name`, mirroring
129+
the scheduler; tools declaring none fall back to the configured global
130+
threshold). Results already carrying a truncation sentinel (prefix or
131+
`<persisted-output>` stub) are skipped — a layer bounded them — and the remaining results are only flagged beyond the combined-pass 2x
132+
tolerance plus a small envelope slack, matching the headroom the scheduler
133+
itself allows. A retained result past that bound means a truncation layer
134+
was bypassed — the counter doubles as a regression alarm.
135+
- Whether oversized outputs are also rendered in UI history (scanned in
136+
`tool_group` items' `resultDisplay`, compared per display against the same
137+
per-tool budget — UI history stores display names, not registry keys, so a
138+
display-name → budget map is built from the tool registry at scan time) and
139+
in compression input (yes by construction, but compression reads history by
140+
reference via `getHistoryShallow`, so no extra copy is held). Phase-1 scope:
141+
only string `resultDisplay` values are measured; structured display objects
142+
(file diffs, ANSI captures, agent result summaries) carry their own
143+
rendering contracts and are not char-comparable in the same way — they are
144+
left for a follow-up PR.
145+
146+
## 6. Alternatives Considered
147+
148+
- **Summarize instead of truncate.** Adds a model round-trip on the hot path
149+
and complicates the privacy model; the pointer-based recovery achieves the
150+
same goal deterministically.
151+
- **Lazy-load history from disk.** Changes the conversation contract and
152+
provider payload shape; the preview + pointer keeps the contract intact.

0 commit comments

Comments
 (0)