fix: suppress tool-call finish_reason planning content

[0xCheetah1]

Summary

• suppress assistant planning text when an upstream choice already ends with finish_reason: "tool_calls"
• preserve the existing tool-call content suppression logic from fix: suppress tool-call planning content #161
• add a regression test for the finish_reason-only edge case while keeping the upstream SSE regression coverage

Why a follow-up PR

PR #161 fixed the common case where providers leaked planning text alongside tool_calls. Live testing uncovered one more structural edge case: some upstreams can mark the turn with finish_reason: "tool_calls" before the tool_calls array is visible at the same inspection point. In that case, planning content could still slip through. This PR hardens that remaining path.

Testing

• targeted regression coverage added in src/proxy.tool-forwarding.test.ts
• live Telegram/OpenClaw testing found intermittent planning leaks that motivated this follow-up hardening

Summary by CodeRabbit

• Bug Fixes

  • Proxy now suppresses assistant prose and preserves a "tool_calls" finish reason when providers indicate tool-call turns, for both streaming and non-streaming responses.

• Tests

  • Added non-stream test ensuring assistant text is blanked when finish_reason signals tool calls; updated an SSE test payload for readability.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 809588ef-df03-4139-8a17-a3c12ac69edb

📥 Commits

Reviewing files that changed from the base of the PR and between 66a821d and c28600d.

📒 Files selected for processing (1)

• src/proxy.tool-forwarding.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/proxy.tool-forwarding.test.ts

---

📝 Walkthrough

Walkthrough

Proxy tests and implementation were updated to treat upstream responses that use finish_reason: "tool_calls" as signaling tool-call turns: assistant prose is suppressed while the finish_reason is preserved for both streaming and non-streaming flows.

Changes

Cohort / File(s)	Summary
Tests — tool-call finish reason src/proxy.tool-forwarding.test.ts	Adds a non-stream test verifying the proxy preserves finish_reason: "tool_calls" while returning an empty choices[0].message.content; reformats an existing SSE test payload for readability.
Proxy response transformation src/proxy.ts	Updates streaming and non-streaming transformation logic to detect finish_reason: "tool_calls" (in addition to tool-call arrays) and blank assistant content for those turns; extends parsing to accept optional finish_reason on choices and sets streamed finish_reason accordingly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix: suppress tool-call planning content #161: Implements similar proxy handling to suppress assistant prose while preserving finish_reason: "tool_calls" and includes related tests.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: suppress tool-call finish_reason planning content' directly and specifically describes the main change: suppressing assistant planning text when finish_reason indicates tool calls, which aligns with the PR objectives and code changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/proxy.tool-forwarding.test.ts (1)

290-341: Consider adding streaming coverage for the new edge case.

This non-streaming test verifies finish_reason: "tool_calls" without a tool_calls array suppresses message.content. The streaming path in src/proxy.ts (line 5072, endsWithToolCalls) handles the same edge case but isn't covered — adding a mirror SSE test would guard against regressions where planning prose leaks through delta.content even when the upstream only signals the tool-call turn via finish_reason.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/proxy.tool-forwarding.test.ts` around lines 290 - 341, Add a streaming
(SSE) test in src/proxy.tool-forwarding.test.ts that mirrors the existing
non-streaming case: configure upstreamResponse to stream events where the final
event has finish_reason: "tool_calls" but no tool_calls array, and assert that
the proxy's SSE stream does not emit planning prose in delta.content (use the
same request body with tools and model as the non-streaming test), read the SSE
events until the stream ends and verify no non-empty assistant delta.content was
forwarded and the final finish_reason sent to the client indicates tool_calls;
focus on exercising the streaming branch handled by the endsWithToolCalls logic
in src/proxy.ts so planning prose doesn't leak via streamed delta.content.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/proxy.tool-forwarding.test.ts`:
- Around line 290-341: Add a streaming (SSE) test in
src/proxy.tool-forwarding.test.ts that mirrors the existing non-streaming case:
configure upstreamResponse to stream events where the final event has
finish_reason: "tool_calls" but no tool_calls array, and assert that the proxy's
SSE stream does not emit planning prose in delta.content (use the same request
body with tools and model as the non-streaming test), read the SSE events until
the stream ends and verify no non-empty assistant delta.content was forwarded
and the final finish_reason sent to the client indicates tool_calls; focus on
exercising the streaming branch handled by the endsWithToolCalls logic in
src/proxy.ts so planning prose doesn't leak via streamed delta.content.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3de9a43a-6db3-4af0-bb99-5bae409ccea9

📥 Commits

Reviewing files that changed from the base of the PR and between 3abfb52 and 66a821d.

📒 Files selected for processing (2)

• src/proxy.tool-forwarding.test.ts
• src/proxy.ts

[1bcMax]

Landed on main as 4f2b85b (rebased on v0.12.165's cache-isolation fix to resolve a minor test-file conflict, contributor authorship preserved). Credited in v0.12.166. Thanks for the quick follow-up!