Skip to content

fix(adapter): honour temperature zero and stop truncating multi-block replies - #8

Merged
hallelx2 merged 4 commits into
mainfrom
halleluyaholudele/hal-524-adapter-correctness
Aug 2, 2026
Merged

fix(adapter): honour temperature zero and stop truncating multi-block replies#8
hallelx2 merged 4 commits into
mainfrom
halleluyaholudele/hal-524-adapter-correctness

Conversation

@hallelx2

@hallelx2 hallelx2 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Two correctness defects in the request/response path. Both affect live traffic today, and both are invisible — no error, no warning, just wrong behaviour.

1. Temperature: 0 was silently discarded

adapter.go gated the option on req.Temperature != 0, so an explicit zero was indistinguishable from "never set" and got dropped. Every such call ran at the provider default, around 1.0.

vectorless-engine sets Temperature: 0 at all nineteen of its call sites — TOC extraction, section summaries, HyDE, query planning, reranking, span extraction, answer synthesis. Every one of them has been sampling hot. That means nondeterministic ingest, nondeterministic citation selection, and JSON that fails to parse more often than it should — each failure costing a retry, which is real money.

Temperature, TopP and Seed are now *float64 / *int: nil means "leave it to the provider", a set pointer is always forwarded. Ptr, Float64 and Int helpers keep call sites readable.

The cache key gains a presence byte per optional field, so an unset value cannot collide with an explicit zero.

2. Only the first content block was read

langchaingo's Anthropic adapter builds one ContentChoice per content block, not per completion candidate:

// llms/anthropic/anthropicllm.go:178
choices := make([]*llms.ContentChoice, len(result.Content))

Reading Choices[0] therefore:

Response blocks Result before this PR
[thinking, text] empty answer
[text, tool_use] tool call silently dropped
[tool_use, tool_use] second call dropped

This is our production path — GLM-4.6 runs through the Anthropic driver against z.ai's compatible gateway — and it defeats the native tool calling added in 69d8b85.

foldChoices now concatenates text, unions tool calls in order, and surfaces thinking on a new Response.ReasoningContent.

The subtle part: every block carries a copy of the same response-level usage, so summing across blocks would have multiplied the reported bill by the block count. Usage is taken from the first block that reports any, never summed — TestUsageCountedOnceAcrossBlocks guards this.

Verification

  • go build ./..., go vet ./... — clean
  • go test ./... — all packages pass, including 8 new tests
  • golangci-lint run — clean (see note below)
  • staticcheck ./... — clean
  • go test -race not run locally — no gcc on this machine, so the race detector is left to CI

Drive-by: two pre-existing lint failures

golangci-lint was already failing on mainpricing/pricing.go lost its map alignment when claude-sonnet-4-20250514 was hand-added, and revive wants a doc comment to open with the bare identifier. Both fixed here because they block CI on any PR, not just this one.

Local gofmt -l also flags several untouched files, but that is a Windows CRLF artifact of the local toolchain — verified byte-identical to gofmt output with diff --strip-trailing-cr. CI runs on Linux and should be clean.

Follow-up

vectorless-engine must migrate its nineteen call sites when it takes this version — tracked on HAL-524. Until then it stays pinned at v0.3.0 and keeps the old behaviour.

Closes HAL-524
Closes HAL-525

Summary by Sourcery

Fix adapter correctness for sampling options and multi-block provider responses, and clean up pricing and cache behaviour.

Bug Fixes:

  • Preserve explicit temperature 0 and other sampling parameters when forwarding requests to providers instead of treating them as unset.
  • Fold multi-block provider responses into a single logical reply so text, tool calls, reasoning traces, and usage are all surfaced correctly and only counted once in billing.
  • Ensure cache keys distinguish between unset and explicitly set sampling values to avoid collisions and incorrect cache hits.

Enhancements:

  • Expose model reasoning content separately from answer content in responses for providers that support it.
  • Add helper constructors for pointer-based request fields to make sampling configuration ergonomics better.

Tests:

  • Add adapter tests covering sampling option forwarding, multi-block response folding, and usage accounting across blocks.

Chores:

  • Fix pre-existing lint issues in pricing configuration alignment and documentation comments.

Summary by CodeRabbit

  • New Features

    • Added support for optional temperature, top-p, and seed sampling settings, including explicit zero values.
    • Responses now expose reasoning content.
    • Added helper functions for creating pointer-based configuration values.
  • Bug Fixes

    • Improved response assembly across multiple provider results, including tool calls, reasoning, finish reasons, and usage metadata.
    • Updated caching to distinguish unset sampling options from explicitly set values.
  • Documentation

    • Clarified warning callback behavior and reformatted pricing entries.

Two defects in the same request/response path, both affecting live
traffic.

1. Temperature 0 was silently discarded. Request.Temperature was a plain
float64 gated by `!= 0`, so a caller asking for 0 was indistinguishable
from one that never set it and the option was dropped — every such call
ran at the provider default, around 1.0. vectorless-engine sets
Temperature: 0 at all nineteen of its call sites (TOC extraction,
section summaries, query planning, reranking, span extraction, answer
synthesis), so the whole engine has been sampling hot: nondeterministic
ingest, nondeterministic citation selection, and JSON that fails to
parse more often than it should, each failure costing a retry.

Temperature, TopP and Seed are now *float64/*int — nil means "leave it
to the provider", a set pointer is always forwarded. Ptr, Float64 and
Int helpers keep call sites readable. The cache key gains a presence
byte per optional field so an unset value cannot collide with an
explicit zero.

2. Only the first content block was read. langchaingo's Anthropic
adapter returns one ContentChoice per *content block*, not per
completion candidate, so a reply of [thinking, text] arrived as two
choices and reading Choices[0] returned an empty answer; a reply of
[text, tool_use] dropped the tool call entirely, defeating the native
tool calling added in 69d8b85. This is our production path — GLM-4.6
runs through the Anthropic driver against z.ai's compatible gateway.

foldChoices now concatenates text, unions tool calls, and surfaces
thinking on the new Response.ReasoningContent. Usage is taken from the
first block that reports any and never summed: every block carries a
copy of the same response-level usage, so adding them would multiply the
reported bill by the block count.

BREAKING CHANGE: Request.Temperature is now *float64. Replace
`Temperature: 0.2` with `Temperature: llmgate.Float64(0.2)`.
Regression coverage for both defects: an explicit temperature of zero
must reach the provider (asserted against a sentinel, since CallOptions
zero-values Temperature and would otherwise hide the bug), and the five
content-block shapes Anthropic actually returns must fold into one
reply with usage counted exactly once.
Both were pre-existing lint failures on main that block CI on any PR:
the price map lost its alignment when claude-sonnet-4-20250514 was hand
added, and revive wants a doc comment to open with the bare identifier.
@sourcery-ai

sourcery-ai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adapter now correctly forwards explicit sampling parameters, distinguishes unset from zero in both request handling and cache keys, and folds multi-block provider responses (including reasoning, tools, and usage) into a single coherent reply, alongside minor pricing and lint cleanups and new regression tests.

Sequence diagram for adapter.Complete with sampling knobs and multi-block folding

sequenceDiagram
    actor Caller
    participant Adapter
    participant LLM as ProviderLLM

    Caller->>Adapter: Complete(ctx, Request)
    Adapter->>Adapter: build opts
    alt Temperature set
        Adapter->>Adapter: llms.WithTemperature(*req.Temperature)
    else Temperature nil
        Adapter->>Adapter: [no temperature option]
    end
    alt TopP set
        Adapter->>Adapter: llms.WithTopP(*req.TopP)
    else TopP nil
        Adapter->>Adapter: [no top_p option]
    end
    alt Seed set
        Adapter->>Adapter: llms.WithSeed(*req.Seed)
    else Seed nil
        Adapter->>Adapter: [no seed option]
    end

    Adapter->>LLM: Call(opts, messages, model)
    LLM-->>Adapter: resp with multiple ContentChoice blocks

    Adapter->>Adapter: folded = foldChoices(resp.Choices)
    Adapter->>Adapter: getInt(folded.genInfo, usageKeys)
    Adapter-->>Caller: Response{Content, ReasoningContent, ToolCalls, Usage}
Loading

File-Level Changes

Change Details Files
Sampling parameters are made optional-with-presence semantics and correctly forwarded to providers and cache keys.
  • Change Request.Temperature from scalar to *float64 and add optional TopP and Seed fields with pointer types.
  • Update adapter Complete to forward Temperature, TopP, and Seed only when non-nil, preserving deterministic temperature=0 instead of treating it as unset.
  • Adjust cacheKey to include presence bytes for optional sampling fields and Seed, and add writeOptFloat helper for hashing optional floats.
  • Add Ptr, Float64, and Int helpers to simplify setting optional fields and update example code to use Float64 for Temperature.
client.go
internal/adapter/adapter.go
middleware/cache/cache.go
examples/smoke/main.go
Provider response choices are folded across content blocks to preserve text, reasoning, tools, and accurate usage reporting.
  • Introduce folded struct and foldChoices function to concatenate content, union tool calls, dedupe reasoning, and select a single finish reason and usage map from multi-block responses.
  • Replace direct use of resp.Choices[0] with foldChoices in Complete and route token accounting through the folded genInfo.
  • Add usageKeys and hasUsage helpers so usage is taken once from the first block that reports it and falls back to the first non-nil GenerationInfo for metadata when none carry usage.
  • Extend Response with ReasoningContent to expose provider reasoning traces separately from answer content.
internal/adapter/adapter.go
client.go
Add targeted regression tests for sampling option semantics and multi-block response folding, including usage accounting behaviors.
  • Add tests that verify temperature=0 reaches the provider, nil leaves provider defaults untouched, and TopP/Seed are only forwarded when set.
  • Add tests that cover Anthropic-style multi-block shapes (thinking+text, text+tool, multiple tools), ensuring content, reasoning, and tool call ordering are preserved.
  • Add tests that enforce usage is counted once across blocks and that a later block with usage can override an earlier no-usage block while still concatenating content.
  • Use a fake model to capture CallOptions and constructed responses for adapter behavior verification.
internal/adapter/adapter_fold_test.go
Address minor lint and formatting issues in pricing and warning documentation.
  • Realign defaultPrices map literals for consistent gofmt-style alignment and readability.
  • Adjust WarnFunc doc comment wording to satisfy revive by starting with the bare identifier name.
pricing/pricing.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@hallelx2, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 48 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e80c3b37-c7f6-4afb-8639-d55087fe0fb1

📥 Commits

Reviewing files that changed from the base of the PR and between 3d591cd and a32ed76.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (1)
  • go.mod
📝 Walkthrough

Walkthrough

The request model now distinguishes unset sampling values from explicit zero values. The adapter forwards optional sampling fields, folds provider choices, exposes reasoning content, and selects usage metadata. Cache keys encode optional sampling values.

Changes

Sampling and response handling

Layer / File(s) Summary
Optional request and response contracts
client.go, examples/smoke/main.go
Request now uses optional Temperature, TopP, and Seed fields. Response exposes ReasoningContent. Pointer helpers support inline option construction.
Provider option forwarding and choice folding
internal/adapter/adapter.go, internal/adapter/adapter_fold_test.go
Complete forwards present sampling values, folds content and reasoning across choices, aggregates tool calls, selects finish reasons and usage metadata, and adds regression coverage.
Optional sampling cache identity
middleware/cache/cache.go
Cache keys encode optional temperature and top-p presence, float bits, and seed values.

Pricing documentation cleanup

Layer / File(s) Summary
Pricing table and callback documentation
pricing/pricing.go
Pricing entries were reformatted without changing model coverage or rates. WarnFunc documentation now describes non-nil invocation.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Request
  participant Adapter
  participant Provider
  participant ChoiceFolder
  participant Response
  Request->>Adapter: optional sampling fields
  Adapter->>Provider: present temperature, top-p, and seed
  Provider-->>Adapter: choices and usage metadata
  Adapter->>ChoiceFolder: provider choices
  ChoiceFolder-->>Adapter: folded response data
  Adapter-->>Response: content, reasoning, tools, finish reason, and usage
Loading
🚥 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 clearly and concisely describes the two primary adapter fixes: preserving zero temperature and combining multi-block responses.
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.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch halleluyaholudele/hal-524-adapter-correctness

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

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues, and left some high level feedback:

  • foldChoices is very Anthropic-specific (multiple content blocks, ThinkingContent dedupe, usage key set) but lives in the generic adapter; consider making its assumptions explicit in the function comment or gating it to Anthropic-only to avoid surprising behavior if other providers start returning multi-choice responses with different semantics.
  • The cacheKey changes correctly differentiate nil vs explicit values for Temperature/TopP/Seed, but there’s no regression coverage for collisions across these cases; consider adding cacheKey tests that assert distinct hashes for nil vs Float64(0) and nil vs Int(0) to lock in the new behavior.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- foldChoices is very Anthropic-specific (multiple content blocks, ThinkingContent dedupe, usage key set) but lives in the generic adapter; consider making its assumptions explicit in the function comment or gating it to Anthropic-only to avoid surprising behavior if other providers start returning multi-choice responses with different semantics.
- The cacheKey changes correctly differentiate nil vs explicit values for Temperature/TopP/Seed, but there’s no regression coverage for collisions across these cases; consider adding cacheKey tests that assert distinct hashes for nil vs Float64(0) and nil vs Int(0) to lock in the new behavior.

## Individual Comments

### Comment 1
<location path="internal/adapter/adapter_fold_test.go" line_range="124-34" />
<code_context>
+func TestFoldChoices(t *testing.T) {
</code_context>
<issue_to_address>
**suggestion (testing):** Extend foldChoices tests to assert FinishReason and reasoning deduplication behaviour

The current table cases cover concatenation and tool-call ordering, but they miss two key behaviours: which StopReason is selected as Response.FinishReason, and that duplicated reasoning (ThinkingContent in both ReasoningContent and GenerationInfo["ThinkingContent"]) is actually deduplicated. Please add expectations for FinishReason (e.g. tool-only → "tool_use", text-only → "end_turn") and a dedicated case where the same reasoning string comes from both sources, asserting that ReasoningContent contains only one copy. This will help catch regressions in the folding logic.
</issue_to_address>

### Comment 2
<location path="internal/adapter/adapter.go" line_range="144" />
<code_context>
+	"OutputTokens", "CompletionTokens", "output_tokens", "completion_tokens",
+}
+
+// foldChoices collapses a provider response into a single reply.
+//
+// langchaingo's Anthropic adapter returns one ContentChoice per *content
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring `foldChoices` into small, focused helper functions and isolating Anthropic-specific behavior to make the adapter logic easier to follow and maintain while preserving semantics.

You can keep the new behavior while reducing complexity by splitting `foldChoices` into smaller helpers and isolating the Anthropic-specific logic. That keeps this adapter readable without changing semantics.

For example:

```go
func foldChoices(choices []*llms.ContentChoice) folded {
	var f folded
	f.content = foldContent(choices)
	f.reasoning = foldReasoning(choices)
	f.toolCalls = foldToolCalls(choices)
	f.finishReason = selectFinishReason(choices)
	f.genInfo = selectGenerationInfo(choices)
	return f
}
```

Then each concern is localized:

```go
func foldContent(choices []*llms.ContentChoice) string {
	var b strings.Builder
	for _, c := range choices {
		if c == nil {
			continue
		}
		b.WriteString(c.Content)
	}
	return b.String()
}
```

Anthropic-specific reasoning handling can be isolated so it’s obvious what’s generic and what isn’t:

```go
func foldReasoning(choices []*llms.ContentChoice) string {
	var b strings.Builder
	seen := map[string]bool{}

	appendReasoning := func(s string) {
		if s == "" || seen[s] {
			return
		}
		seen[s] = true
		b.WriteString(s)
	}

	for _, c := range choices {
		if c == nil {
			continue
		}
		// Generic reasoning
		appendReasoning(c.ReasoningContent)

		// Anthropic-specific duplication fix, clearly scoped
		if s, ok := c.GenerationInfo["ThinkingContent"].(string); ok {
			appendReasoning(s)
		}
	}
	return b.String()
}
```

Usage selection can also be made clearer and self-contained:

```go
func selectGenerationInfo(choices []*llms.ContentChoice) map[string]any {
	for _, c := range choices {
		if c == nil || c.GenerationInfo == nil {
			continue
		}
		if hasUsage(c.GenerationInfo) {
			return c.GenerationInfo
		}
	}
	// Fallback: any non-nil map for other metadata
	for _, c := range choices {
		if c != nil && c.GenerationInfo != nil {
			return c.GenerationInfo
		}
	}
	return nil
}
```

This keeps all the current behavior (including Anthropic reasoning dedup and usage selection) but dramatically flattens the control flow in `foldChoices`, making the generic adapter easier to follow and maintain without moving logic into provider-specific adapters.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

t.Fatalf("Complete: %v", err)
}
return fm.gotOpts
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Extend foldChoices tests to assert FinishReason and reasoning deduplication behaviour

The current table cases cover concatenation and tool-call ordering, but they miss two key behaviours: which StopReason is selected as Response.FinishReason, and that duplicated reasoning (ThinkingContent in both ReasoningContent and GenerationInfo["ThinkingContent"]) is actually deduplicated. Please add expectations for FinishReason (e.g. tool-only → "tool_use", text-only → "end_turn") and a dedicated case where the same reasoning string comes from both sources, asserting that ReasoningContent contains only one copy. This will help catch regressions in the folding logic.

"OutputTokens", "CompletionTokens", "output_tokens", "completion_tokens",
}

// foldChoices collapses a provider response into a single reply.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider refactoring foldChoices into small, focused helper functions and isolating Anthropic-specific behavior to make the adapter logic easier to follow and maintain while preserving semantics.

You can keep the new behavior while reducing complexity by splitting foldChoices into smaller helpers and isolating the Anthropic-specific logic. That keeps this adapter readable without changing semantics.

For example:

func foldChoices(choices []*llms.ContentChoice) folded {
	var f folded
	f.content = foldContent(choices)
	f.reasoning = foldReasoning(choices)
	f.toolCalls = foldToolCalls(choices)
	f.finishReason = selectFinishReason(choices)
	f.genInfo = selectGenerationInfo(choices)
	return f
}

Then each concern is localized:

func foldContent(choices []*llms.ContentChoice) string {
	var b strings.Builder
	for _, c := range choices {
		if c == nil {
			continue
		}
		b.WriteString(c.Content)
	}
	return b.String()
}

Anthropic-specific reasoning handling can be isolated so it’s obvious what’s generic and what isn’t:

func foldReasoning(choices []*llms.ContentChoice) string {
	var b strings.Builder
	seen := map[string]bool{}

	appendReasoning := func(s string) {
		if s == "" || seen[s] {
			return
		}
		seen[s] = true
		b.WriteString(s)
	}

	for _, c := range choices {
		if c == nil {
			continue
		}
		// Generic reasoning
		appendReasoning(c.ReasoningContent)

		// Anthropic-specific duplication fix, clearly scoped
		if s, ok := c.GenerationInfo["ThinkingContent"].(string); ok {
			appendReasoning(s)
		}
	}
	return b.String()
}

Usage selection can also be made clearer and self-contained:

func selectGenerationInfo(choices []*llms.ContentChoice) map[string]any {
	for _, c := range choices {
		if c == nil || c.GenerationInfo == nil {
			continue
		}
		if hasUsage(c.GenerationInfo) {
			return c.GenerationInfo
		}
	}
	// Fallback: any non-nil map for other metadata
	for _, c := range choices {
		if c != nil && c.GenerationInfo != nil {
			return c.GenerationInfo
		}
	}
	return nil
}

This keeps all the current behavior (including Anthropic reasoning dedup and usage selection) but dramatically flattens the control flow in foldChoices, making the generic adapter easier to follow and maintain without moving logic into provider-specific adapters.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/adapter/adapter.go`:
- Around line 170-186: Update the content assembly in foldChoices to prevent
adjacent non-empty ContentChoice.Content values from merging: insert a space
boundary when the current non-empty text follows previously written
non-whitespace-separated content, while preserving existing handling for nil,
empty, tool, and reasoning blocks.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 244063b6-b24c-4af0-bbdb-73ae2730ae7d

📥 Commits

Reviewing files that changed from the base of the PR and between 69d8b85 and 3d591cd.

📒 Files selected for processing (6)
  • client.go
  • examples/smoke/main.go
  • internal/adapter/adapter.go
  • internal/adapter/adapter_fold_test.go
  • middleware/cache/cache.go
  • pricing/pricing.go

Comment on lines +170 to +186
for _, c := range choices {
if c == nil {
continue
}
content.WriteString(c.Content)
appendReasoning(c.ReasoningContent)
if s, ok := c.GenerationInfo["ThinkingContent"].(string); ok {
appendReasoning(s)
}
f.toolCalls = append(f.toolCalls, fromLangchainToolCalls(c)...)
if c.StopReason != "" {
f.finishReason = c.StopReason
}
if f.genInfo == nil && hasUsage(c.GenerationInfo) {
f.genInfo = c.GenerationInfo
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Can Anthropic Claude API return multiple separate text content blocks in one response (citations feature)?

💡 Result:

Yes, the Anthropic Claude API can return multiple separate text content blocks in a single response [1][2]. The Claude API defines responses as an array of content blocks [3][2]. When the citations feature is enabled, a single response from the model may include multiple text blocks [1][4]. Each text block can contain a claim generated by the model, accompanied by a citations field that lists the specific sources supporting that claim [4][5]. Key details regarding the response structure include: * Multiple Content Blocks: The content field in an API response is an array [3][2]. A single response can contain multiple blocks, including combinations of text blocks, tool use blocks, or thinking blocks [2]. * Citations Structure: When citations are enabled, individual text content blocks may have an associated citations field [6][5]. This field provides the evidence grounding the content within that specific block [1][7]. * Minimal Citable Unit: Text blocks serve as the minimal citable unit [4][6]. The citations indicate the specific range within source documents—such as character indices for plain text, page numbers for PDFs, or content block indices for custom documents—that supports the text in that block [1][8]. Robust integrations are designed to iterate through the entire response.content array rather than assuming a single block, ensuring all content and associated citations are captured [2].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## Locate adapter files"
fd -a 'adapter.*\.go$' . | sed 's#^\./##' | sort

echo
echo "## internal/adapter/adapter.go outline"
ast-grep outline internal/adapter/adapter.go || true

echo
echo "## Relevant adapter.go lines 130-205"
sed -n '130,205p' internal/adapter/adapter.go | nl -ba -v130

echo
echo "## Search for foldChoices and ContentChoice"
rg -n "foldChoices|ContentChoice|GenerationInfo|ThinkingContent|ReasoningContent|ToolCall|toolCalls" internal/adapter/adapter.go internal/adapter/*_test.go 2>/dev/null || true

echo
echo "## Find langchain dependencies"
rg -n "github.com/tmc/langchaingo|llms\.|TextContent|Citation" go.mod vendor 2>/dev/null || true
if [ -d go.mod ]; then
  sed -n '1,220p' go.mod
fi

echo
echo "## Behavioral model of current foldChoices content concatenation"
python3 - <<'PY'
choices = [
    {"Content": "Block one.", "GenerationInfo": {}, "StopReason": ""},
    {"Content": "Block two with citation. ", "GenerationInfo": {}, "StopReason": ""},
    {"Content": "Block three.", "GenerationInfo": {}, "StopReason": ""},
]
content = []
for c in choices:
    content.append(c["Content"])
print("concat:", "".join(content), "| len=", len("".join(content)))
for a, b in zip(content, content[1:]):
    merged = a + b
    if content and " " not in merged.strip() or (a != "" and b != "" and a[-1].isalnum() and b[0].isalnum()):
        print("adjacent non empty content:", repr(a), "+", repr(b), "=>", repr(merged))
PY

Repository: hallelx2/llmgate

Length of output: 1801


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## Relevant adapter.go lines 130-205"
awk 'NR>=130 && NR<=205 { printf "%5d\t%s\n", NR, $0 }' internal/adapter/adapter.go

echo
echo "## Fold-choice tests"
awk 'NR>=1 && NR<=260 { printf "%5d\t%s\n", NR, $0 }' internal/adapter/adapter_fold_test.go

echo
echo "## Provider/provider-related text in tests"
rg -n "Anthropic|cl|TextContent|Citation|Text" internal/adapter/adapter_fold_test.go internal/adapter/adapter_tools_test.go internal/ 2>/dev/null || true

echo
echo "## Behavioral model of current foldChoices content concatenation"
python3 - <<'PY'
choices = [
    {"Content": "Block one."},
    {"Content": "Block two with citation. "},
    {"Content": "Block three."},
]
merged = "".join(c["Content"] for c in choices)
print("merged:", repr(merged))
print("merges_without_boundary:", "one.Block" in merged or "citation. Block" not in merged)
PY

Repository: hallelx2/llmgate

Length of output: 15807


🌐 Web query:

langchaingo Anthropic adapter TextContent citations ContentChoice

💡 Result:

In the langchaingo project's Anthropic adapter, TextContent and ContentChoice are key components used to handle structured interactions with Anthropic models [1][2]. TextContent is a struct defined within the anthropic internal client (specifically under llms/anthropic/internal/anthropicclient) used to represent text blocks returned by the model [1][2]. It typically follows this structure: type TextContent struct { Type string json:"type" Text string json:"text" } ContentChoice is a struct defined in the main llms package that represents a single completion choice returned by an LLM [1][3]. When the Anthropic adapter processes responses, it maps Anthropic's content blocks (such as TextContent or ToolUseContent) into llms.ContentChoice objects [1]. This mapping is critical for ensuring that both text responses and tool calls are correctly interpreted by the broader langchaingo framework [1]. Regarding citations, Anthropic models support a feature that allows the inclusion of source citations in responses [4]. While Anthropic's native API allows passing citations via search_result or document content blocks, the langchaingo adapter's handling of these features involves translating between internal LangChain representations and Anthropic's specific JSON structures [5]. It is important to note that the langchaingo implementation of the Anthropic adapter has faced technical challenges regarding multi-part content handling [6][7]. Specifically, earlier versions encountered issues where assistant messages containing both text and tool calls were not fully processed, as the adapter sometimes only inspected the first part of the message (Parts) [6]. Efforts have been made to update the adapter to iterate through all message parts to ensure that all content types, including text and tool usage, are correctly preserved and serialized [6].

Citations:


🌐 Web query:

github.com tmc langchaingo internal anthropicclient messages.go TextContent citations

💡 Result:

In the langchaingo library, specifically within the github.com/tmc/langchaingo/llms/anthropic/internal/anthropicclient package, TextContent is a defined structure used to represent text-based content blocks in Anthropic API messages [1]. The TextContent struct is defined as follows: type TextContent struct { Type string json:"type" Text string json:"text" CacheControl *CacheControl json:"cache_control,omitempty" } It implements the Content interface by providing a GetType method, which returns the content type (typically "text") [1][2]. The package utilizes this structure to handle messages sent to and received from the Anthropic API. Recent updates to the library have focused on improving the handling of mixed content (such as combinations of text and tool calls) in assistant messages, addressing issues where only the first part of a message was processed [3][4][5][6]. These changes ensure that when the Anthropic API returns responses containing both text and tool use blocks, all parts are correctly iterated and serialized [3][4].

Citations:


Handle adjacent non-empty ContentChoice entries with a boundary.

Anthropic responses can contain multiple text content blocks, and content.WriteString(c.Content) currently joins every ContentChoice.Content without a separator. If foldChoices receives two adjacent non-text tool/thought blocks and then two adjacent non-empty text blocks, the response text will merge without a space. Add a space separator before non-empty content or add a case that expects two non-whitespace-separated blocks.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/adapter/adapter.go` around lines 170 - 186, Update the content
assembly in foldChoices to prevent adjacent non-empty ContentChoice.Content
values from merging: insert a space boundary when the current non-empty text
follows previously written non-whitespace-separated content, while preserving
existing handling for nil, empty, tool, and reasoning blocks.

Two pre-existing CI failures, both unrelated to the adapter fixes in
this branch but both blocking any PR against main.

`go mod tidy` was not idempotent: internal/adapter imports tiktoken-go
directly, but go.mod still listed it as `// indirect`, so the ubuntu
job's tidiness check failed on every run.

govulncheck flagged reachable CVEs in the transitive tree pulled in by
langchaingo — grpc transport, x/text norm and idna, x/net http2, and
OpenTelemetry baggage extraction. Bumping x/net, x/text, otel and grpc
clears all of them; what govulncheck still reports is confined to the
Go standard library and resolves on the toolchain patch CI installs.
@hallelx2
hallelx2 merged commit 0382140 into main Aug 2, 2026
7 checks passed
@hallelx2
hallelx2 deleted the halleluyaholudele/hal-524-adapter-correctness branch August 2, 2026 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant