Skip to content

fix(adk): preserve streaming tool output during reduction#1090

Merged
N3kox merged 3 commits into
cloudwego:mainfrom
Zhang-986:codex/streaming-reduction-nonblocking
Jul 7, 2026
Merged

fix(adk): preserve streaming tool output during reduction#1090
N3kox merged 3 commits into
cloudwego:mainfrom
Zhang-986:codex/streaming-reduction-nonblocking

Conversation

@Zhang-986

@Zhang-986 Zhang-986 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • use a default streaming-specific truncation notice with no tail preview, so default streaming truncation does not wait for EOF before notifying truncation
  • forward chunks only until MaxLengthForTrunc, emit the notice once the threshold is crossed, then drain remaining output for offload without forwarding it
  • keep non-streaming and tool-specific/custom TruncHandler behavior unchanged
  • cover streamable/enhanced streamable nonblocking behavior plus default streaming terminal, error, drain, and consumer-close paths

Fixes #1064.

Tests

  • go test ./adk/middlewares/reduction -run 'TestReductionMiddlewareTrunc/.*default.*streamable.*(terminal cases|nonblocking)' -count=1
  • go test ./adk/middlewares/reduction -count=1
  • go test ./adk/middlewares/reduction -coverprofile=/tmp/eino-reduction-after2.cover -covermode=count -count=1
  • local patch coverage estimate for adk/middlewares/reduction/reduction.go: 217/227 coverable added lines hit, about 95.59%
  • wrapper coverage: wrapDefaultStreamableTruncation 94.4%, wrapDefaultEnhancedStreamableTruncation 95.5%
  • go test ./adk/... -count=1
  • git diff --check

Note: the latest fork workflow runs are currently waiting for maintainer approval (action_required), so Codecov has not recalculated for commit 95ac44d yet.

@CLAassistant

CLAassistant commented Jun 19, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.19087% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.24%. Comparing base (e8832e2) to head (32f937f).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
adk/middlewares/reduction/reduction.go 94.64% 7 Missing and 5 partials ⚠️
adk/middlewares/reduction/consts.go 88.23% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1090      +/-   ##
==========================================
+ Coverage   83.11%   83.24%   +0.12%     
==========================================
  Files         162      162              
  Lines       23134    23397     +263     
==========================================
+ Hits        19228    19476     +248     
- Misses       2640     2649       +9     
- Partials     1266     1272       +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Zhang-986 Zhang-986 force-pushed the codex/streaming-reduction-nonblocking branch from 172fd49 to 7a86258 Compare June 24, 2026 15:15
@Zhang-986

Copy link
Copy Markdown
Contributor Author

Updated the PR to address the Codecov failures by adding targeted coverage for the new default streaming truncation paths.

What changed:

  • Added terminal-path tests for default streamable truncation: short/no truncation, source stream error, trunc handler error, no-trunc decision, offload error, nil trunc result, and non-text result parts.
  • Added the same terminal-path coverage for enhanced streamable truncation, including concat-error handling.
  • Kept production code unchanged.

Local checks:

  • go test ./adk/middlewares/reduction -run 'TestReductionMiddlewareTrunc/.*default.*truncation|TestReductionMiddlewareTrunc/.*nonblocking' -count=1
  • go test ./adk/middlewares/reduction -coverprofile=/tmp/eino-reduction.cover -covermode=count -count=1
  • go test ./adk/middlewares/reduction -count=1
  • go test ./adk/... -count=1
  • git diff --check

Local diff coverage estimate for adk/middlewares/reduction/reduction.go: 140/148 coverable added lines hit, about 94.6%.

@N3kox

N3kox commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

@Zhang-986 hi, thanks for your tackling #1064 - the blocking behavior is real and worth fixing

However, my concern is that this PR seems to address the symptom rather than the root cause. As long as the default truncation format is preview_first + notice + preview_last, the streaming path will inherently need EOF to construct preview_last. In that sense, draining the stream is a consequence of the current prompt shape, not just an implementation detail.

That’s why I’m hesitant about this approach: it streams the prefix live, but once the threshold is crossed it still waits for EOF and then emits the full truncation notice, which includes preview_first. So the consumer effectively gets the head content twice, and the final persisted/history content diverges from the invoke / non-streaming path for the same tool.

I wonder if the cleaner fix is at the prompt layer: use a streaming-specific truncation format with a full-length head preview plus a trailing notice, and no tail preview. Then truncation can remain fully incremental: stream until the threshold, stop, emit the notice, and avoid both EOF waiting and duplicated content. Such as:

[chunk_1]
[chunk_2]
...
[chunk_n]                 ← threshold reached; stop forwarding here
                          (chunk_n+1 … chunk_m: collected & written to file, never sent)
[trunc_notify_chunk("output truncated: {original_size} total, first {preview_size} shown above. Full output saved to: {file_path}")]
EOF

Happy to discuss if I’m missing a constraint, but I’d be more comfortable with a prompt-level fix here.

@Zhang-986 Zhang-986 force-pushed the codex/streaming-reduction-nonblocking branch from 7a86258 to aecf6ae Compare June 25, 2026 15:23
@Zhang-986

Copy link
Copy Markdown
Contributor Author

Implemented the prompt-level/default streaming fix:

  • default streamable and enhanced streamable truncation now use a streaming-specific notice with no tail preview, so the notice can be emitted before source EOF
  • chunks are forwarded only up to MaxLengthForTrunc; after that the wrapper emits the notice, drains the remaining source output, and writes the full content to the backend without forwarding the tail
  • non-streaming truncation and tool-specific/custom TruncHandler behavior are unchanged

Verification:

  • go test ./adk/middlewares/reduction -run 'TestReductionMiddlewareTrunc/.*(nonblocking|terminal cases)' -count=1
  • go test ./adk/middlewares/reduction -count=1
  • go test ./adk/middlewares/reduction -coverprofile=/tmp/eino-reduction.cover -covermode=count -count=1
  • local patch coverage estimate: 85.84% (200/233 changed statements)
  • go test ./adk/... -count=1
  • git diff --check

@Zhang-986

Zhang-986 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Updated the branch with commit 95ac44d to cover the remaining meaningful default streaming truncation terminal paths.

What changed in the latest commit:

  • added coverage for draining tail chunks after the truncation notice
  • added backend write-error forwarding for streamable and enhanced streamable wrappers
  • added enhanced concat-error coverage before notice emission
  • added consumer-close early-return coverage for short, prefix, and notice sends

Local verification:

  • go test ./adk/middlewares/reduction -count=1
  • go test ./adk/... -count=1
  • git diff --check
  • local patch coverage estimate for adk/middlewares/reduction/reduction.go: 217/227 coverable added lines hit, about 95.59%

The latest fork workflow runs are currently waiting for maintainer approval (action_required), so Codecov has not recalculated this commit yet.

@N3kox

N3kox commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@Zhang-986
Hi, thanks again for your contribution. I made some improvements based on your commits, please take a look at it if you got time.

  • Refactored default streamable truncation to use StreamReaderWithConvert handlers instead of an extra goroutine + pipe wrapper. And applied the same flow to enhanced streamable tool results.
  • Preserved the source-error fallback behavior: if a non-EOF source error arrives after truncation has started, suppressed original chunks are replayed first, then the original source error is returned.
  • Moved final truncation notice/offload work to source EOF, so the wrapper can keep accumulating the full output before offloading.
  • Improved boundary handling for offload/path/backend/write failures: these are now rendered in the truncation notice instead of interrupting the stream as errors.
  • Updated stream truncation prompt formatting and tests.

@N3kox N3kox merged commit 747023b into cloudwego:main Jul 7, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

reduction middleware WrapStreamableToolCall blocks until stream is fully consumed, defeating streaming

4 participants