Skip to content

Commit cc7462d

Browse files
fix(clients): capture inline <think> reasoning in vLLM + Ollama (#113)
Reasoning models emit chain-of-thought either in a structured field or inline in content (often <think>...</think>). VLLMClient.send() only read the structured `reasoning` field, so when a model put its thinking inline it was silently dropped -- no REASONING message downstream, breaking reasoning replay. OllamaClient had a raw-content fallback but never extracted <think> tags. Bring both clients to parity with LlamafileClient using the shared forge.prompts.think_tags.extract_think_tags helper: - vLLM: rework _resolve_reasoning to a single (reasoning, content) form used identically by send() and send_stream() -- structured field -> <think> extraction -> raw content. Strip <think> from TextResponse in both paths. - Ollama: extract <think> tags when the structured `thinking` field is absent; strip <think> from TextResponse in both paths. Reasoning capture stays gated on self._think; TextResponse tag-stripping is unconditional, matching LlamafileClient. Adds 10 regression tests (6 non-streaming, 4 streaming). Closes #110. Claude-Session: https://claude.ai/code/session_01EpuVYCYeb1DhWfynCVyA6a Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1830706 commit cc7462d

4 files changed

Lines changed: 218 additions & 22 deletions

File tree

src/forge/clients/ollama.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from forge.clients.sampling_defaults import apply_sampling_defaults
1313
from forge.core.workflow import LLMResponse, TextResponse, ToolCall, ToolSpec
1414
from forge.errors import BackendError, ThinkingNotSupportedError
15+
from forge.prompts.think_tags import extract_think_tags
1516

1617
_THINK_HEURISTIC_KEYWORDS = ("reason", "think")
1718

@@ -120,12 +121,17 @@ def _resolve_reasoning(
120121
) -> str | None:
121122
"""Gate reasoning capture on _think flag.
122123
123-
When _think is False, discard all reasoning.
124-
When True: prefer thinking field, fall back to content.
124+
When _think is False, discard all reasoning. When True: prefer the
125+
structured ``thinking`` field; if absent, extract ``<think>`` tags from
126+
content; finally fall back to the raw content (an instruct model
127+
narrating before its tool call). Mirrors LlamafileClient.
125128
"""
126129
if not self._think:
127130
return None
128-
return thinking or content or None
131+
if thinking:
132+
return thinking
133+
think, _ = extract_think_tags(content)
134+
return think or content or None
129135

130136
def _record_usage(self, data: dict[str, Any]) -> None:
131137
"""Extract token usage from an Ollama response."""
@@ -214,7 +220,10 @@ async def send(
214220
for i, tc in enumerate(tool_calls)
215221
]
216222

217-
return TextResponse(content=msg.get("content", ""))
223+
# No tool calls: strip inline thinking so the TextResponse carries
224+
# clean content (parity with LlamafileClient).
225+
_, content = extract_think_tags(msg.get("content", ""))
226+
return TextResponse(content=content)
218227

219228
async def send_stream(
220229
self,
@@ -321,7 +330,8 @@ async def _iter_stream(
321330
content = msg.get("content", "")
322331
if content:
323332
accumulated_content += content
324-
final = TextResponse(content=accumulated_content)
333+
_, text = extract_think_tags(accumulated_content)
334+
final = TextResponse(content=text)
325335
yield StreamChunk(type=ChunkType.FINAL, response=final)
326336
else:
327337
tool_calls = msg.get("tool_calls")

src/forge/clients/vllm.py

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from forge.clients.sampling_defaults import apply_sampling_defaults
3030
from forge.core.workflow import LLMResponse, TextResponse, ToolCall, ToolSpec
3131
from forge.errors import BackendError
32+
from forge.prompts.think_tags import extract_think_tags
3233

3334

3435
class VLLMClient:
@@ -163,23 +164,26 @@ def _record_usage(self, data: dict[str, Any]) -> None:
163164
total_tokens=usage.get("total_tokens", 0),
164165
)
165166

166-
def _resolve_reasoning(
167-
self, message_or_accum: dict[str, Any] | str, accumulated_content: str = "",
168-
) -> str | None:
169-
"""Extract reasoning, gated on _think.
170-
171-
vLLM 0.21 returns reasoning in the ``reasoning`` field of the
172-
assistant message when ``--reasoning-parser`` is enabled at server
173-
boot. If thinking is disabled or the field is empty, return None.
174-
175-
Accepts either a message dict (from send()) or an accumulated
176-
reasoning string (from send_stream()).
167+
def _resolve_reasoning(self, reasoning: str, content: str) -> str | None:
168+
"""Build final reasoning from the structured field and content, gated
169+
on _think.
170+
171+
vLLM 0.21 returns reasoning in the ``reasoning`` field of the assistant
172+
message when ``--reasoning-parser`` is enabled at server boot. When
173+
that parser is absent — or doesn't split a given model's output — the
174+
thinking instead arrives inline in ``content`` (often wrapped in
175+
``<think>...</think>``). To avoid silently dropping it (issue #110) and
176+
to keep send() and send_stream() in lockstep with LlamafileClient, fall
177+
back to ``<think>``-tag extraction and then to the raw content when the
178+
structured field is empty. Both call sites pass the same (reasoning,
179+
content) pair, so the two paths resolve identically.
177180
"""
178181
if not self._think:
179182
return None
180-
if isinstance(message_or_accum, dict):
181-
return message_or_accum.get("reasoning") or None
182-
return message_or_accum or accumulated_content or None
183+
if reasoning:
184+
return reasoning
185+
think, _ = extract_think_tags(content)
186+
return think or content or None
183187

184188
async def send(
185189
self,
@@ -227,10 +231,16 @@ async def send(
227231
if tool_calls:
228232
return self._parse_tool_calls(
229233
tool_calls,
230-
reasoning=self._resolve_reasoning(message),
234+
reasoning=self._resolve_reasoning(
235+
message.get("reasoning") or "", message.get("content") or "",
236+
),
231237
)
232238

233-
return TextResponse(content=message.get("content") or "")
239+
# No tool calls: strip any inline thinking — reasoning is only useful
240+
# attached to a ToolCall; a TextResponse carries clean content (parity
241+
# with LlamafileClient.send()).
242+
_, content = extract_think_tags(message.get("content") or "")
243+
return TextResponse(content=content)
234244

235245
async def send_stream(
236246
self,
@@ -334,7 +344,9 @@ async def send_stream(
334344
),
335345
)
336346
else:
337-
final = TextResponse(content=accumulated_content)
347+
# Strip inline thinking from the final text for parity with send().
348+
_, text = extract_think_tags(accumulated_content)
349+
final = TextResponse(content=text)
338350
yield StreamChunk(type=ChunkType.FINAL, response=final)
339351

340352
@staticmethod

tests/unit/test_ollama_client.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,41 @@ async def test_think_false_discards_reasoning(self) -> None:
225225
assert isinstance(result, list)
226226
assert result[0].reasoning is None
227227

228+
@pytest.mark.asyncio
229+
async def test_extracts_think_tags_from_content_with_tool_call(self) -> None:
230+
"""<think> tags inline in content are extracted (not the raw tagged
231+
string), when there is no structured thinking field."""
232+
client = _make_client(think=True)
233+
client._http.post.return_value = _mock_response({
234+
"message": {
235+
"role": "assistant",
236+
"content": "<think>price first</think>",
237+
"tool_calls": [
238+
{"function": {"name": "get_pricing", "arguments": {"part": "X"}}}
239+
],
240+
}
241+
})
242+
result = await client.send(
243+
[{"role": "user", "content": "test"}], tools=[_make_spec()]
244+
)
245+
assert isinstance(result, list)
246+
assert result[0].reasoning == "price first"
247+
248+
@pytest.mark.asyncio
249+
async def test_think_tags_stripped_from_text_response(self) -> None:
250+
"""A bare text reply has <think> tags stripped from its content."""
251+
client = _make_client()
252+
client._http.post.return_value = _mock_response({
253+
"message": {
254+
"role": "assistant",
255+
"content": "<think>pondering</think>Hello there.",
256+
"tool_calls": [],
257+
}
258+
})
259+
result = await client.send([{"role": "user", "content": "test"}])
260+
assert isinstance(result, TextResponse)
261+
assert result.content == "Hello there."
262+
228263
@pytest.mark.asyncio
229264
async def test_think_true_explicit(self) -> None:
230265
"""think=True explicitly → always in request body."""
@@ -499,6 +534,52 @@ async def test_streaming_captures_reasoning_from_deltas(self) -> None:
499534
assert isinstance(final.response, list)
500535
assert final.response[0].reasoning == "Let me think..."
501536

537+
@pytest.mark.asyncio
538+
async def test_streaming_extracts_think_tags_from_content_with_tool_call(self) -> None:
539+
"""#110 (streaming): inline <think> in streamed content (no thinking
540+
deltas) is extracted onto the FINAL tool call."""
541+
client = _make_client(think=True)
542+
lines = [
543+
json.dumps({"message": {"role": "assistant", "content": "<think>price "}, "done": False}),
544+
json.dumps({"message": {"role": "assistant", "content": "first</think>"}, "done": False}),
545+
json.dumps({
546+
"message": {
547+
"role": "assistant",
548+
"content": "",
549+
"tool_calls": [
550+
{"function": {"name": "get_pricing", "arguments": {"part": "X"}}}
551+
],
552+
},
553+
"done": True,
554+
}),
555+
]
556+
client._http.stream.return_value = _MockStreamResponse(lines)
557+
chunks = []
558+
async for chunk in client.send_stream(
559+
[{"role": "user", "content": "test"}], tools=[_make_spec()]
560+
):
561+
chunks.append(chunk)
562+
final = [c for c in chunks if c.type == ChunkType.FINAL][0]
563+
assert isinstance(final.response, list)
564+
assert final.response[0].reasoning == "price first"
565+
566+
@pytest.mark.asyncio
567+
async def test_streaming_strips_think_tags_from_text_response(self) -> None:
568+
"""A streamed bare text reply has <think> tags stripped from FINAL."""
569+
client = _make_client()
570+
lines = [
571+
json.dumps({"message": {"role": "assistant", "content": "<think>pondering</think>"}, "done": False}),
572+
json.dumps({"message": {"role": "assistant", "content": "Hello there."}, "done": False}),
573+
json.dumps({"message": {"role": "assistant", "content": ""}, "done": True}),
574+
]
575+
client._http.stream.return_value = _MockStreamResponse(lines)
576+
chunks = []
577+
async for chunk in client.send_stream([{"role": "user", "content": "test"}]):
578+
chunks.append(chunk)
579+
final = [c for c in chunks if c.type == ChunkType.FINAL][0]
580+
assert isinstance(final.response, TextResponse)
581+
assert final.response.content == "Hello there."
582+
502583
@pytest.mark.asyncio
503584
async def test_streaming_thinking_preferred_over_content(self) -> None:
504585
"""Streamed thinking tokens are preferred over content for reasoning."""

tests/unit/test_vllm_client.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,58 @@ async def test_think_false_discards_reasoning(self) -> None:
179179
assert isinstance(result, list)
180180
assert result[0].reasoning is None
181181

182+
@pytest.mark.asyncio
183+
async def test_extracts_think_tags_from_content_with_tool_call(self) -> None:
184+
"""#110: thinking inline in content (no `reasoning` field) is captured."""
185+
client = _make_client(think=True)
186+
client._http.post.return_value = _mock_response(
187+
_tool_call_response(content="<think>check the weather first</think>"),
188+
)
189+
result = await client.send(
190+
[{"role": "user", "content": "x"}], tools=[_make_spec()],
191+
)
192+
assert isinstance(result, list)
193+
assert result[0].reasoning == "check the weather first"
194+
195+
@pytest.mark.asyncio
196+
async def test_reasoning_field_preferred_over_content_tags(self) -> None:
197+
"""Structured `reasoning` field wins over <think> tags in content."""
198+
client = _make_client(think=True)
199+
client._http.post.return_value = _mock_response(
200+
_tool_call_response(reasoning="structured", content="<think>inline</think>"),
201+
)
202+
result = await client.send(
203+
[{"role": "user", "content": "x"}], tools=[_make_spec()],
204+
)
205+
assert isinstance(result, list)
206+
assert result[0].reasoning == "structured"
207+
208+
@pytest.mark.asyncio
209+
async def test_think_tags_stripped_from_text_response(self) -> None:
210+
"""A bare text reply has <think> tags stripped from its content."""
211+
client = _make_client()
212+
client._http.post.return_value = _mock_response(
213+
_text_response("<think>pondering</think>The answer is 42."),
214+
)
215+
result = await client.send([{"role": "user", "content": "x"}])
216+
assert isinstance(result, TextResponse)
217+
assert result.content == "The answer is 42."
218+
219+
@pytest.mark.asyncio
220+
async def test_thinking_only_text_response_empty_after_strip(self) -> None:
221+
"""Thinking-only reply (no answer, no tool call) strips to empty content.
222+
223+
Matches LlamafileClient; the empty TextResponse then rides the existing
224+
ResponseValidator retry path (covered in the validator tests).
225+
"""
226+
client = _make_client()
227+
client._http.post.return_value = _mock_response(
228+
_text_response("<think>just thinking, no answer yet</think>"),
229+
)
230+
result = await client.send([{"role": "user", "content": "x"}])
231+
assert isinstance(result, TextResponse)
232+
assert result.content == ""
233+
182234
@pytest.mark.asyncio
183235
async def test_usage_recorded(self) -> None:
184236
client = _make_client()
@@ -381,6 +433,47 @@ async def test_accumulates_reasoning_across_deltas(self) -> None:
381433
assert isinstance(result, list)
382434
assert result[0].reasoning == "Let me think... "
383435

436+
@pytest.mark.asyncio
437+
async def test_stream_extracts_think_tags_from_content_with_tool_call(self) -> None:
438+
"""#110 (streaming): inline <think> in streamed content (no reasoning
439+
deltas) is captured on the FINAL tool call."""
440+
client = _make_client(think=True)
441+
client._http.stream.return_value = _MockStreamResponse([
442+
_sse({"choices": [{"delta": {"content": "<think>inline "}}]}),
443+
_sse({"choices": [{"delta": {"content": "plan</think>"}}]}),
444+
_sse({"choices": [{"delta": {
445+
"tool_calls": [{
446+
"index": 0,
447+
"function": {"name": "get_weather", "arguments": '{"city": "P"}'}
448+
}],
449+
}}]}),
450+
"data: [DONE]",
451+
])
452+
chunks = []
453+
async for chunk in client.send_stream(
454+
[{"role": "user", "content": "x"}], tools=[_make_spec()],
455+
):
456+
chunks.append(chunk)
457+
result = [c for c in chunks if c.type == ChunkType.FINAL][0].response
458+
assert isinstance(result, list)
459+
assert result[0].reasoning == "inline plan"
460+
461+
@pytest.mark.asyncio
462+
async def test_stream_strips_think_tags_from_text_response(self) -> None:
463+
"""A streamed bare text reply has <think> tags stripped from FINAL."""
464+
client = _make_client()
465+
client._http.stream.return_value = _MockStreamResponse([
466+
_sse({"choices": [{"delta": {"content": "<think>pondering</think>"}}]}),
467+
_sse({"choices": [{"delta": {"content": "The answer is 42."}}]}),
468+
"data: [DONE]",
469+
])
470+
chunks = []
471+
async for chunk in client.send_stream([{"role": "user", "content": "x"}]):
472+
chunks.append(chunk)
473+
result = [c for c in chunks if c.type == ChunkType.FINAL][0].response
474+
assert isinstance(result, TextResponse)
475+
assert result.content == "The answer is 42."
476+
384477
@pytest.mark.asyncio
385478
async def test_non_200_raises_backend_error(self) -> None:
386479
client = _make_client()

0 commit comments

Comments
 (0)