fix: preserve reasoning_content for thinking models in multi-turn conversations

[siciyuan404]

Summary

This PR fixes an issue where thinking models (e.g., GLM-Z1, Kimi K2.5) fail during multi-turn conversations with tool calls.

Problem

When using thinking models that require
easoning_content\ (推理内容), the API would return errors like:

• \ hinking is enabled but reasoning_content is missing in assistant tool call message\
• \已启用思考功能，但索引X处的助手工具调用消息中缺少推理内容\

This happened because the
easoning_content\ field was not being preserved when building assistant messages with tool calls.

Changes

1. Add ReasoningContent field to types (\pkg/providers/types.go)

• Add \ReasoningContent\ field to \LLMResponse\ struct
• Add \ReasoningContent\ field to \Message\ struct

2. Parse reasoning_content from API response (\pkg/providers/http_provider.go)

• Parse
  easoning_content\ from the API response for thinking models

3. Preserve ReasoningContent in agent loop (\pkg/agent/loop.go)

• Include \ReasoningContent\ when building assistant messages with tool calls

4. Preserve ReasoningContent in toolloop (\pkg/tools/toolloop.go)

• Include \ReasoningContent\ when building assistant messages in subagent tool calls

Testing

Tested with GLM-Z1 and Kimi K2.5 thinking models, confirming that multi-turn conversations with tool calls now work correctly.

Commits

• 39998a6: fix: preserve reasoning_content for thinking models in multi-turn conversations
• 53ddf35: fix: preserve ReasoningContent in toolloop for thinking models

[lxowalle]

Hi @siciyuan404, thanks for the pr, could you please fix the ci(pr-check) error

[Leeaandrob]

@Zepan Preserves reasoning_content for thinking models (DeepSeek, etc.) in multi-turn conversations. Without this fix, the reasoning chain gets lost between turns.

Recommendation: Merge. +23/-17, important for thinking model support which is increasingly popular.

[Leeaandrob]

PR #159 Deep Review — @siciyuan404

Hey @siciyuan404, thanks for identifying and fixing this real-world bug with thinking models. The problem statement is well-documented and the fix is correctly scoped.

However, I have a few concerns — one critical regarding merge ordering with an in-progress roadmap item, and some gaps in test coverage.

---

Roadmap Context

This PR touches the same files as Roadmap Item #283 ("Refactor Provider Architecture", priority: HIGH, status: In Progress) which is implemented in PR #213. Both PRs modify types.go and http_provider.go. Merge order is critical — whichever merges second will face significant conflicts.

This PR also aligns with Roadmap Item #295 ("Intelligent Model Routing for Cost & Performance Optimization") since thinking model support is a model-specific behavior that should be handled at the protocol/routing layer.

---

Summary of Findings

HIGH (Should Fix)

• H1: Direct merge conflict with PR #213 (Roadmap #283) — both modify types.go and http_provider.go
• H2: Zero test coverage for the new reasoning_content parsing and propagation
• H3: ClaudeProvider doesn't handle Claude's "thinking" content blocks — related gap worth addressing here

MEDIUM

• M1: The reasoning_content field name is provider-specific (Zhipu/Kimi/DeepSeek convention) — should be documented
• M2: No CI status visible on this PR

POSITIVE

1. Small, focused fix — only +23/-17 lines across 4 files
2. Correctly uses omitempty to avoid sending empty field to non-thinking APIs
3. Propagates consistently in both agent/loop.go and tools/toolloop.go
4. Well-written PR description with clear problem statement and testing evidence
5. The fix correctly targets only OpenAI-compatible providers (not Anthropic, which uses a different mechanism)

---

Verdict: REQUEST CHANGES

What needs to change before merge:

• Coordinate merge order with PR #213 (they both modify types.go and http_provider.go)
• Add at least a unit test for reasoning_content parsing in http_provider.go
• Consider adding "thinking" block support to parseClaudeResponse for parity

Estimated effort: ~2-3 hours. Happy to re-review once addressed.

[Leeaandrob]

[H1 — HIGH: Merge Conflict with PR #213] @siciyuan404 — This file (types.go) is also modified by PR #213 ("Refactor providers by protocol family"), which is the implementation of Roadmap Item #283 (priority: HIGH, status: In Progress). PR #213 also modifies http_provider.go and creates new sub-packages (openai_compat/, anthropic/) with their own type definitions.

If PR #213 merges first: Your changes here will need to be applied to the openai_compat/provider.go types as well (and the type-conversion layer in the refactored http_provider.go).

If this PR merges first: PR #213 will need to propagate ReasoningContent to all three type locations.

Recommendation: Coordinate with @jmahotiedu (PR #213 author) on merge order. Given that this is a small bug fix and #213 is a larger refactor, I'd suggest merging this first and having #213 pick up the new field.

[Leeaandrob]

[M1 — MEDIUM: Provider-Specific Field] @siciyuan404 — The reasoning_content JSON field name is a convention used specifically by Chinese LLM providers (Zhipu GLM-Z1, Kimi K2.5, DeepSeek R1). Other providers may use different names:

• OpenAI o1/o3: Uses a different mechanism (reasoning tokens in usage, not a content field)
• Anthropic Claude: Uses "thinking" content blocks via the SDK

The comment says "e.g., GLM-Z1" which is accurate but incomplete. Consider updating to:

ReasoningContent string `json:"reasoning_content,omitempty"` // For thinking models (Zhipu GLM-Z1, Kimi K2.5, DeepSeek R1)

Also, if other OpenAI-compatible providers start using different field names for reasoning output, this approach won't scale. Worth adding a note in the code or docs about this limitation.

[Leeaandrob]

[H2 — HIGH: No Test Coverage] @siciyuan404 — This is the core parsing logic for the new feature, but there's no unit test that validates reasoning_content is correctly parsed from an API response.

The existing TestHTTPProvider_parseResponse (or similar) should have a new test case like:

func TestParseResponse_ReasoningContent(t *testing.T) {
    body := `{"choices":[{"message":{"content":"answer","reasoning_content":"thinking process"},"finish_reason":"stop"}]}`
    p := &HTTPProvider{}
    resp, err := p.parseResponse([]byte(body))
    if err != nil {
        t.Fatalf("parseResponse() error: %v", err)
    }
    if resp.ReasoningContent != "thinking process" {
        t.Errorf("ReasoningContent = %q, want %q", resp.ReasoningContent, "thinking process")
    }
}

Also need a test that verifies ReasoningContent is empty/omitted for non-thinking models.

Without tests, this fix could regress silently in future refactors (especially with PR #213 restructuring these files).

[Leeaandrob]

[Positive] @siciyuan404 — Good catch propagating ReasoningContent from the response into the assistant message. This is the critical path that was causing the API errors. The fact that you also caught the same pattern in toolloop.go shows thoroughness.

[Leeaandrob]

[Positive] @siciyuan404 — Correctly mirrored the fix from agent/loop.go. Both message-building paths (main agent loop and subagent tool loop) now preserve ReasoningContent, which means thinking models work in both direct conversations and tool-call chains. Good completeness.

[Leeaandrob]

[H3 — HIGH: Claude Provider Gap] @siciyuan404 — You correctly added ReasoningContent propagation for the HTTP (OpenAI-compatible) provider. However, the ClaudeProvider in claude_provider.go has a related gap:

parseClaudeResponse() (line 155-172) only handles block.Type == "text" and block.Type == "tool_use". Claude models with extended thinking produce block.Type == "thinking" content blocks, which are silently dropped.

While this is a pre-existing issue and Claude uses a different mechanism than reasoning_content, this PR is the natural place to add parity. The fix would be:

case "thinking":
    tb := block.AsThinking()
    // Store in ReasoningContent for consistency
    reasoningContent += tb.Thinking

This would give PicoClaw consistent thinking-model support across both protocol families (OpenAI-compat and Anthropic).

If you'd prefer to keep this PR focused, at minimum please open a follow-up issue tracking this gap.

[xiaket]

@siciyuan404 do you mind rebase and resolve the conflicts? Thanks!