diff --git a/docs/CONTEXT_PRESERVATION_BUGS.md b/docs/CONTEXT_PRESERVATION_BUGS.md new file mode 100644 index 00000000..44ca0d5e --- /dev/null +++ b/docs/CONTEXT_PRESERVATION_BUGS.md @@ -0,0 +1,89 @@ +# Context Preservation Bugs for API Calls + +Bugs in preserving interleaved **thinking**, **tool calls**, and **text** when converting between **Anthropic** format (API surface) and **NVIDIA NIM** (backend provider). NIM uses the OpenAI-compatible API format internally. + +## Summary + +| Bug | Location | Impact | Test | +|-----|----------|--------|------| +| 1. Assistant message interleaving lost | `message_converter._convert_assistant_message` | Thinking+text order flattened to [all thinking, all text, tool_calls] | `test_convert_assistant_interleaved_order_preserved` | +| 2. User message text/tool_result order reversed | `message_converter._convert_user_message` | User text emitted after tool results instead of before | `test_convert_user_message_text_before_tool_result_order` | +| 3. Response interleaved think tags collapsed | `response.convert_response` + `extract_think_content` | Multiple `...` blocks merged into one; text between them preserved but thinking blocks lose interleaving | `test_interleaved_think_tags_in_content_preserved` | + +--- + +## Bug 1: Assistant Message Interleaving Lost + +**File:** `providers/nvidia_nim/utils/message_converter.py` +**Function:** `_convert_assistant_message` + +**Current behavior:** Collects all `thinking` blocks into `reasoning_parts`, all `text` blocks into `text_parts`, all `tool_use` into `tool_calls`. Output order is always: `[all thinking] [all text] [tool_calls]`. + +**Example:** Input blocks `[thinking, text, thinking, tool_use]` +- **Expected:** Content string `first\n\nHere is the answer.\n\nsecond` with tool_calls at end +- **Actual:** Content string `first\nsecond\n\nHere is the answer.` (all thinking merged first) + +**API constraint:** NIM (OpenAI-compatible) format has `content: string` and `tool_calls: array`. Tool calls cannot be interleaved within content. We can only preserve thinking↔text order within the content string. + +**Fix direction:** Iterate blocks in order; for each block, append to content string: if thinking → add `...`; if text → add text. Tool calls stay at end. + +--- + +## Bug 2: User Message Text/Tool Result Order Reversed + +**File:** `providers/nvidia_nim/utils/message_converter.py` +**Function:** `_convert_user_message` + +**Current behavior:** Emits `tool_result` blocks immediately to `result`, then appends user text at the end. Order becomes: `[tool, tool, ..., user]`. + +**Example:** Input blocks `[text, tool_result]` (user says "Please use this result:", then provides tool output) +- **Expected:** `[user, tool]` +- **Actual:** `[tool, user]` + +**Anthropic convention:** User typically provides context first, then tool results. Reversing can confuse models that expect user text before tool results. + +**Fix direction:** Emit blocks in original order: text → user message, tool_result → tool message. + +--- + +## Bug 3: Response Interleaved Think Tags Collapsed + +**File:** `providers/nvidia_nim/response.py` + `providers/nvidia_nim/utils/think_parser.py` +**Function:** `convert_response`, `extract_think_content` + +**Current behavior:** `extract_think_content` uses `re.findall(r"(.*?)", text)` and joins all matches into one thinking string, then strips all tags from content. Remaining text is one block. + +**Example:** Content `firstmiddlesecond` +- **Expected:** `[thinking("first"), text("middle"), thinking("second")]` +- **Actual:** `[thinking("first\nsecond"), text("middle")]` + +**Fix direction:** Parse content sequentially (e.g. iterate with `re.finditer` or use a stateful parser) and emit blocks in order: thinking, text, thinking, text, etc. + +--- + +## Streaming Path + +The **streaming** response path in `providers/nvidia_nim/client.py` uses `ThinkTagParser` and `HeuristicToolParser`, which yield chunks in order. Interleaving is preserved during streaming. The bugs above affect: + +- **Request path:** Converting Anthropic messages → NIM (outbound API calls) +- **Non-streaming response path:** Converting NIM response → Anthropic format + +--- + +## Failing Tests (Reproduction) + +```bash +uv run pytest tests/test_converter.py::test_convert_assistant_interleaved_order_preserved -v +uv run pytest tests/test_converter.py::test_convert_user_message_text_before_tool_result_order -v +uv run pytest tests/test_response_conversion.py::TestConvertResponse::test_interleaved_think_tags_in_content_preserved -v +``` + +All three tests now pass after the fixes below. + +## Fixes Applied + +1. **Assistant interleaving:** `_convert_assistant_message` now iterates blocks in order and appends each thinking/text block to `content_parts` sequentially. Tool calls remain at the end (API constraint). + +2. **User message order:** `_convert_user_message` now uses `flush_text()` before each tool_result so user text is emitted first when it precedes tool results. + +3. **Response think tags:** Added `extract_think_content_interleaved()` that uses `re.finditer` to emit blocks in order. `convert_response` uses it when `reasoning_content` is absent. diff --git a/providers/nvidia_nim/response.py b/providers/nvidia_nim/response.py index ecd187d0..3ddf6539 100644 --- a/providers/nvidia_nim/response.py +++ b/providers/nvidia_nim/response.py @@ -4,7 +4,7 @@ import json import uuid from typing import Any -from .utils import map_stop_reason, extract_think_content +from .utils import map_stop_reason, extract_think_content_interleaved def convert_response(response_json: dict, original_request: Any) -> dict: @@ -27,16 +27,21 @@ def convert_response(response_json: dict, original_request: Any) -> dict: if reasoning: content.append({"type": "thinking", "thinking": reasoning}) - # Extract text content (with think tag handling) + # Extract text content (with think tag handling, preserving interleaving) if message.get("content"): raw_content = message["content"] if isinstance(raw_content, str): if not reasoning: - think_content, raw_content = extract_think_content(raw_content) - if think_content: - content.append({"type": "thinking", "thinking": think_content}) - if raw_content: - content.append({"type": "text", "text": raw_content}) + for block_type, block_content in extract_think_content_interleaved( + raw_content + ): + if block_type == "thinking": + content.append({"type": "thinking", "thinking": block_content}) + else: + content.append({"type": "text", "text": block_content}) + else: + if raw_content.strip(): + content.append({"type": "text", "text": raw_content.strip()}) elif isinstance(raw_content, list): for item in raw_content: if isinstance(item, dict) and item.get("type") == "text": diff --git a/providers/nvidia_nim/utils/__init__.py b/providers/nvidia_nim/utils/__init__.py index b16addce..5e909b6a 100644 --- a/providers/nvidia_nim/utils/__init__.py +++ b/providers/nvidia_nim/utils/__init__.py @@ -6,6 +6,7 @@ from .think_parser import ( ContentType, ContentChunk, extract_think_content, + extract_think_content_interleaved, ) from .heuristic_tool_parser import HeuristicToolParser from .message_converter import ( @@ -23,6 +24,7 @@ __all__ = [ "ContentType", "ContentChunk", "extract_think_content", + "extract_think_content_interleaved", "AnthropicToOpenAIConverter", "get_block_attr", "get_block_type", diff --git a/providers/nvidia_nim/utils/message_converter.py b/providers/nvidia_nim/utils/message_converter.py index 45d5b57d..0bb3b859 100644 --- a/providers/nvidia_nim/utils/message_converter.py +++ b/providers/nvidia_nim/utils/message_converter.py @@ -48,18 +48,18 @@ class AnthropicToOpenAIConverter: @staticmethod def _convert_assistant_message(content: List[Any]) -> List[Dict[str, Any]]: - """Convert assistant message blocks.""" - text_parts = [] - tool_calls = [] - reasoning_parts = [] + """Convert assistant message blocks, preserving interleaved thinking+text order.""" + content_parts: List[str] = [] + tool_calls: List[Dict[str, Any]] = [] for block in content: block_type = get_block_type(block) if block_type == "text": - text_parts.append(get_block_attr(block, "text", "")) + content_parts.append(get_block_attr(block, "text", "")) elif block_type == "thinking": - reasoning_parts.append(get_block_attr(block, "thinking", "")) + thinking = get_block_attr(block, "thinking", "") + content_parts.append(f"\n{thinking}\n") elif block_type == "tool_use": tool_input = get_block_attr(block, "input", {}) tool_calls.append( @@ -75,18 +75,7 @@ class AnthropicToOpenAIConverter: } ) - # Merge everything into content for NIM/Mistral compatibility - # Anthropic 'thinking' blocks are converted to tags - actual_content = [] - if reasoning_parts: - # Join reasoning parts and handle as a separate block - reasoning_str = "\n".join(reasoning_parts) - actual_content.append(f"\n{reasoning_str}\n") - - if text_parts: - actual_content.append("\n".join(text_parts)) - - content_str = "\n\n".join(actual_content) + content_str = "\n\n".join(content_parts) # Ensure content is never an empty string for assistant messages # NIM (especially Mistral models) requires non-empty content if there are no tool calls @@ -104,9 +93,14 @@ class AnthropicToOpenAIConverter: @staticmethod def _convert_user_message(content: List[Any]) -> List[Dict[str, Any]]: - """Convert user message blocks (including tool results).""" - result = [] - text_parts = [] + """Convert user message blocks (including tool results), preserving order.""" + result: List[Dict[str, Any]] = [] + text_parts: List[str] = [] + + def flush_text() -> None: + if text_parts: + result.append({"role": "user", "content": "\n".join(text_parts)}) + text_parts.clear() for block in content: block_type = get_block_type(block) @@ -114,6 +108,7 @@ class AnthropicToOpenAIConverter: if block_type == "text": text_parts.append(get_block_attr(block, "text", "")) elif block_type == "tool_result": + flush_text() tool_content = get_block_attr(block, "content", "") if isinstance(tool_content, list): tool_content = "\n".join( @@ -130,9 +125,7 @@ class AnthropicToOpenAIConverter: } ) - if text_parts: - result.append({"role": "user", "content": "\n".join(text_parts)}) - + flush_text() return result @staticmethod diff --git a/providers/nvidia_nim/utils/think_parser.py b/providers/nvidia_nim/utils/think_parser.py index 2ceb9f64..73327876 100644 --- a/providers/nvidia_nim/utils/think_parser.py +++ b/providers/nvidia_nim/utils/think_parser.py @@ -2,7 +2,7 @@ import re from dataclasses import dataclass -from typing import Optional, Tuple, Iterator +from typing import List, Optional, Tuple, Iterator from enum import Enum @@ -173,6 +173,8 @@ def extract_think_content(text: str) -> Tuple[Optional[str], str]: Extract thinking content from text (non-streaming). Returns: (thinking_content, remaining_text) + Merges all think blocks and strips them from text. Use extract_think_content_interleaved + when interleaved order must be preserved. """ think_pattern = re.compile(r"(.*?)", re.DOTALL) matches = think_pattern.findall(text) @@ -183,3 +185,25 @@ def extract_think_content(text: str) -> Tuple[Optional[str], str]: return thinking, remaining return None, text + + +def extract_think_content_interleaved(text: str) -> List[Tuple[str, str]]: + """ + Parse content and return blocks in order, preserving interleaving of + ... and text. + + Returns: [(type, content), ...] where type is "thinking" or "text". + """ + blocks: List[Tuple[str, str]] = [] + pattern = re.compile(r"(.*?)", re.DOTALL) + last_end = 0 + for m in pattern.finditer(text): + before = text[last_end : m.start()].strip() + if before: + blocks.append(("text", before)) + blocks.append(("thinking", m.group(1))) + last_end = m.end() + after = text[last_end:].strip() + if after: + blocks.append(("text", after)) + return blocks diff --git a/tests/test_converter.py b/tests/test_converter.py index e3475a3f..9930aa2c 100644 --- a/tests/test_converter.py +++ b/tests/test_converter.py @@ -141,20 +141,10 @@ def test_convert_user_message_mixed_text_and_tool_result(): messages = [MockMessage("user", content)] result = AnthropicToOpenAIConverter.convert_messages(messages) - # Expected: Tool messages come first? Or order is preserved? - # Logic: loop over blocks. if tool_result -> append to result. if text -> append to text_parts. - # finally if text_parts -> append new user message. - # So tool results come first in the list, then the text message. - # Wait, looking at code: - # for block in content: - # if tool_result: result.append(...) - # if text: text_parts.append(...) - # if text_parts: result.append(...) - # Yes, tool results first, then user text. - + # Order is preserved: user text first, then tool result. assert len(result) == 2 - assert result[0] == {"role": "tool", "tool_call_id": "tool_789", "content": "42"} - assert result[1] == {"role": "user", "content": "Here is the result:"} + assert result[0] == {"role": "user", "content": "Here is the result:"} + assert result[1] == {"role": "tool", "tool_call_id": "tool_789", "content": "42"} # --- Message Conversion Tests: Assistant --- @@ -344,6 +334,56 @@ def test_convert_tool_use_none_input(): assert "tool_calls" in result[0] +def test_convert_assistant_interleaved_order_preserved(): + """Interleaved thinking, text, tool_use should preserve thinking+text order in content. + + Bug: Current implementation collects all thinking, then all text, then tool_calls. + Original order [thinking, text, thinking, tool_use] becomes [all thinking, all text, tool_calls], + losing the interleaving. Content string should reflect original block order for thinking+text. + Tool calls stay at end (API constraint). + """ + content = [ + MockBlock(type="thinking", thinking="First thought."), + MockBlock(type="text", text="Here is the answer."), + MockBlock(type="thinking", thinking="Second thought."), + MockBlock(type="tool_use", id="call_1", name="search", input={"q": "x"}), + ] + messages = [MockMessage("assistant", content)] + result = AnthropicToOpenAIConverter.convert_messages(messages) + + assert len(result) == 1 + msg = result[0] + # Expected: thinking1, text, thinking2 in that order within content; tool_calls at end + expected_content = ( + "\nFirst thought.\n\n\nHere is the answer.\n\n\nSecond thought.\n" + ) + assert msg["content"] == expected_content, ( + f"Interleaved order lost. Got: {msg['content']!r}" + ) + assert len(msg["tool_calls"]) == 1 + + +def test_convert_user_message_text_before_tool_result_order(): + """User message with text then tool_result should preserve order: user text first, then tool. + + Bug: Current implementation emits tool_result immediately, then user text at end. + Anthropic order is typically: user says something, then provides tool results. + """ + content = [ + MockBlock(type="text", text="Please use this result:"), + MockBlock(type="tool_result", tool_use_id="t1", content="42"), + ] + messages = [MockMessage("user", content)] + result = AnthropicToOpenAIConverter.convert_messages(messages) + + assert len(result) == 2 + # Expected: user text first, then tool result + assert result[0]["role"] == "user" + assert result[0]["content"] == "Please use this result:" + assert result[1]["role"] == "tool" + assert result[1]["tool_call_id"] == "t1" + + def test_convert_multiple_tool_results(): """Multiple tool results in a single user message.""" content = [ diff --git a/tests/test_response_conversion.py b/tests/test_response_conversion.py index 1d638993..79693ce0 100644 --- a/tests/test_response_conversion.py +++ b/tests/test_response_conversion.py @@ -194,3 +194,23 @@ class TestConvertResponse: resp = _make_response() result = convert_response(resp, _make_request(model="claude-3")) assert result["model"] == "claude-3" + + def test_interleaved_think_tags_in_content_preserved(self): + """Interleaved ... and text in content should preserve order. + + Bug: extract_think_content uses findall and joins all matches, then strips + tags from content. So "axb" becomes thinking="a\\nb", remaining="x". + Output is [thinking, text] instead of [thinking, text, thinking]. + """ + resp = _make_response(content="firstmiddlesecond") + result = convert_response(resp, _make_request()) + types = [b["type"] for b in result["content"]] + # Expected: thinking, text, thinking (interleaved order) + assert types == ["thinking", "text", "thinking"], ( + f"Interleaved order lost. Got types: {types}" + ) + thinking_blocks = [b for b in result["content"] if b["type"] == "thinking"] + assert thinking_blocks[0]["thinking"] == "first" + assert thinking_blocks[1]["thinking"] == "second" + text_blocks = [b for b in result["content"] if b["type"] == "text"] + assert text_blocks[0]["text"] == "middle"