Merge pull request #116 from The-Obstacle-Is-The-Way/fix/p0-aifunction-serialization

docs/bugs/P0_HUGGINGFACE_TOOL_CALLING_BROKEN.md

@@ -0,0 +1,173 @@
+# P0 Bug: HuggingFace Free Tier Tool Calling Broken
+
+**Severity**: P0 (Critical) - Free Tier cannot perform multi-turn tool-based research
+**Status**: PARTIALLY RESOLVED - Bug #1 FIXED, Bug #2 requires upstream fix
+**Discovered**: 2025-12-01
+**Investigator**: Claude Code (Systematic First-Principles Analysis)
+**Last Updated**: 2025-12-01
+
+## Executive Summary
+
+The HuggingFace Free Tier had two critical bugs preventing end-to-end tool-based research:
+
+1. **Bug #1 (FIXED)**: Conversation history serialization missing `tool_calls` and `tool_call_id`
+2. **Bug #2 (UPSTREAM)**: Microsoft Agent Framework produces repr strings instead of message text
+
+## Current Status
+
+| Bug | Status | Location | Fix |
+|-----|--------|----------|-----|
+| #1 History Serialization | ✅ **FIXED** | `src/clients/huggingface.py` | Commit `809ad60` |
+| #2 Framework Repr Bug | ⏳ **UPSTREAM** | `agent_framework/_workflows/_magentic.py` | [Issue #2562](https://github.com/microsoft/agent-framework/issues/2562) |
+
+---
+
+## BUG #1: Conversation History Serialization ✅ FIXED
+
+### What Was Wrong
+`_convert_messages()` didn't serialize `tool_calls` (for assistant messages) or `tool_call_id` (for tool messages).
+
+### The Fix (Commit `809ad60`)
+Updated `_convert_messages()` in `src/clients/huggingface.py:71-121` to:
+1. Extract `FunctionCallContent` from `msg.contents` → `tool_calls` array
+2. Extract `FunctionResultContent` from `msg.contents` → `tool_call_id`
+3. Properly format for HuggingFace/OpenAI API
+
+### Verification
+```python
+# Before fix: BadRequestError on multi-turn
+# After fix: Multi-turn conversations work
+
+# The message format is now correct:
+{
+    "role": "assistant",
+    "content": "",
+    "tool_calls": [{"id": "call_123", "type": "function", "function": {...}}]
+}
+```
+
+---
+
+## BUG #2: Framework Message Corruption ⏳ UPSTREAM
+
+### Symptom
+`MagenticAgentMessageEvent.message.text` contains:
+```text
+'<agent_framework._types.ChatMessage object at 0x10c394210>'
+```
+
+### Root Cause (CONFIRMED)
+**File**: `agent_framework/_workflows/_magentic.py` line ~1799
+
+```python
+async def _invoke_agent(self, ctx, ...) -> ChatMessage:
+    # ...
+    if messages and len(messages) > 0:
+        last: ChatMessage = messages[-1]
+        text = last.text or str(last)  # <-- BUG: str(last) gives repr!
+        msg = ChatMessage(role=role, text=text, author_name=author)
+```
+
+**Why it happens**:
+1. `ChatMessage.text` property only extracts `TextContent` items
+2. Tool-call-only messages have empty `.text` (returns `""`)
+3. `"" or str(last)` evaluates to `str(last)`
+4. `ChatMessage` has no `__str__` method → default Python repr
+
+### Impact Assessment
+
+| Aspect | Impact | Critical? |
+|--------|--------|-----------|
+| UI Display | Shows garbage instead of agent output | YES for UX |
+| Logging | Can't debug what agents did | YES for debugging |
+| Tool Execution | Tools ARE being called (middleware works) | NO - Works |
+| Research Completion | Manager may not track progress properly | MAYBE - Unclear |
+
+**Observed behavior**: Research loops often reach max rounds without synthesis. The Manager keeps saying "no progress" even though tools ARE being called. This COULD be:
+1. The repr bug affecting Manager's understanding
+2. Qwen 72B not handling tool message format well
+3. Unrelated orchestration issue
+
+### Upstream Issue Filed
+**GitHub Issue**: [microsoft/agent-framework#2562](https://github.com/microsoft/agent-framework/issues/2562)
+
+**Suggested fixes in issue**:
+1. **Minimal**: `text = last.text or ""`
+2. **Better UX**: Format tool calls for display
+3. **Best**: Add `__str__` to `ChatMessage` class
+
+### Workaround (Implemented in `advanced.py`)
+We modified `_extract_text()` in `advanced.py` to extract tool call names from `.contents` when text is empty or looks like a repr:
+
+```python
+def _extract_text(self, message: Any) -> str:
+    # ... existing logic with repr filtering ...
+
+    # Workaround: Extract tool call info when text is repr/empty
+    if hasattr(message, "contents") and message.contents:
+        tool_names = [
+            f"[Tool: {c.name}]"
+            for c in message.contents
+            if hasattr(c, "name")  # FunctionCallContent
+        ]
+        if tool_names:
+            return " ".join(tool_names)
+
+    return ""
+```
+
+**Decision**: Implemented locally to fix display and logging while we wait for upstream fix.
+
+---
+
+## Verification Matrix (Updated)
+
+| Component | Status | Notes |
+|-----------|--------|-------|
+| Tool Serialization | ✅ WORKS | `_convert_tools()` |
+| Tool Call Parsing | ✅ WORKS | `_parse_tool_calls()` |
+| History Serialization | ✅ **FIXED** | `_convert_messages()` |
+| Middleware Decorators | ✅ **FIXED** | `@use_function_invocation` etc. |
+| Event Display | ❌ UPSTREAM | Shows repr - framework bug |
+| End-to-End Research | ⚠️ UNCLEAR | Needs testing after upstream fix |
+
+---
+
+## Files Changed
+
+### Fixed (Commit `809ad60`)
+- `src/clients/huggingface.py`
+  - `_convert_messages()` - Now serializes `tool_calls` and `tool_call_id`
+  - Added `@use_function_invocation`, `@use_observability`, `@use_chat_middleware` decorators
+  - Added `__function_invoking_chat_client__ = True` marker
+
+### Also Fixed
+- `src/orchestrators/advanced.py` - `_extract_text()` now filters repr strings AND extracts tool call names
+
+---
+
+## Related Upstream Issues
+
+| Issue | Title | Status | Relevance |
+|-------|-------|--------|-----------|
+| [#2562](https://github.com/microsoft/agent-framework/issues/2562) | Repr string bug (OUR ISSUE) | OPEN | Direct cause |
+| [#1366](https://github.com/microsoft/agent-framework/issues/1366) | Thread corruption - unexecuted tool calls | OPEN | Same area |
+| [#2410](https://github.com/microsoft/agent-framework/issues/2410) | OpenAI client splits content/tool_calls | OPEN | Related bug |
+
+---
+
+## Next Steps
+
+1. **Monitor**: Watch for response to [Issue #2562](https://github.com/microsoft/agent-framework/issues/2562)
+2. **Test**: Run end-to-end research tests to see if Bug #2 actually blocks completion
+3. **Optional**: Implement workaround in `_extract_text()` if display is critical
+4. **Contribute**: Consider submitting PR to fix `_magentic.py` line 1799
+
+---
+
+## References
+
+- [HuggingFace Chat Completion API - Tool Use](https://huggingface.co/docs/huggingface_hub/package_reference/inference_client#huggingface_hub.InferenceClient.chat_completion)
+- [OpenAI Function Calling](https://platform.openai.com/docs/guides/function-calling)
+- [Microsoft Agent Framework Repository](https://github.com/microsoft/agent-framework)
+- [Our Upstream Issue #2562](https://github.com/microsoft/agent-framework/issues/2562)

src/clients/huggingface.py

@@ -6,6 +6,7 @@ an OpenAI API key.
 """
 
 import asyncio
+import json
 from collections.abc import AsyncIterable, MutableSequence
 from functools import partial
 from typing import Any, cast
@@ -17,8 +18,13 @@ from agent_framework import (
     ChatOptions,
     ChatResponse,
     ChatResponseUpdate,
+    FinishReason,
+    Role,
 )
-from agent_framework._types import FunctionCallContent
+from agent_framework._middleware import use_chat_middleware
+from agent_framework._tools import use_function_invocation
+from agent_framework._types import FunctionCallContent, FunctionResultContent
+from agent_framework.observability import use_observability
 from huggingface_hub import InferenceClient
 
 from src.utils.config import settings
@@ -26,9 +32,16 @@ from src.utils.config import settings
 logger = structlog.get_logger()
 
 
+@use_function_invocation
+@use_observability
+@use_chat_middleware
 class HuggingFaceChatClient(BaseChatClient):  # type: ignore[misc]
     """Adapter for HuggingFace Inference API with full function calling support."""
 
+    # Marker to tell agent_framework that this client supports function calling
+    # Without this, the framework warns and ignores tools
+    __function_invoking_chat_client__ = True
+
     def __init__(
         self,
         model_id: str | None = None,
@@ -58,16 +71,72 @@ class HuggingFaceChatClient(BaseChatClient):  # type: ignore[misc]
     def _convert_messages(self, messages: MutableSequence[ChatMessage]) -> list[dict[str, Any]]:
         """Convert framework messages to HuggingFace format."""
         hf_messages: list[dict[str, Any]] = []
+
+        # Track call_id -> tool_name mapping for tool result messages
+        # Assistant messages with tool_calls come before tool result messages
+        call_id_to_name: dict[str, str] = {}
+
         for msg in messages:
-            # Basic conversion - extend as needed for multi-modal
-            content = msg.text or ""
             # msg.role can be string or enum - extract .value for enums
-            # str(Role.USER) -> "Role.USER" (wrong), Role.USER.value -> "user" (correct)
             if hasattr(msg.role, "value"):
                 role_str = str(msg.role.value)
             else:
                 role_str = str(msg.role)
-            hf_messages.append({"role": role_str, "content": content})
+
+            content_str = msg.text or ""
+            tool_calls = []
+            tool_call_id = None
+            tool_name = None
+
+            # Process contents for tool calls and results
+            if msg.contents:
+                for item in msg.contents:
+                    if isinstance(item, FunctionCallContent):
+                        # This is an assistant message invoking a tool
+                        # Track call_id -> name for later tool result messages
+                        call_id_to_name[item.call_id] = item.name
+                        tool_calls.append(
+                            {
+                                "id": item.call_id,
+                                "type": "function",
+                                "function": {
+                                    "name": item.name,
+                                    "arguments": (
+                                        item.arguments
+                                        if isinstance(item.arguments, str)
+                                        else json.dumps(item.arguments)
+                                    ),
+                                },
+                            }
+                        )
+                    elif isinstance(item, FunctionResultContent):
+                        # This is a tool result message
+                        role_str = "tool"
+                        tool_call_id = item.call_id
+                        # Look up tool name from prior FunctionCallContent
+                        tool_name = call_id_to_name.get(item.call_id)
+                        # For tool results, JSON-encode structured data
+                        # HuggingFace/OpenAI expects string content
+                        if item.result is None:
+                            content_str = ""
+                        elif isinstance(item.result, str):
+                            content_str = item.result
+                        else:
+                            content_str = json.dumps(item.result)
+
+            message_dict: dict[str, Any] = {"role": role_str, "content": content_str}
+
+            if tool_calls:
+                message_dict["tool_calls"] = tool_calls
+
+            if tool_call_id:
+                message_dict["tool_call_id"] = tool_call_id
+                # Add name field if we tracked it (required by some APIs)
+                if tool_name:
+                    message_dict["name"] = tool_name
+
+            hf_messages.append(message_dict)
+
         return hf_messages
 
     def _convert_tools(self, tools: list[Any] | None) -> list[dict[str, Any]] | None:
@@ -108,12 +177,7 @@ class HuggingFaceChatClient(BaseChatClient):  # type: ignore[misc]
         return json_tools if json_tools else None
 
     def _parse_tool_calls(self, message: Any) -> list[FunctionCallContent]:
-        """Parse HuggingFace tool_calls into framework FunctionCallContent.
-
-        HF returns tool_calls as:
-            [ChatCompletionOutputToolCall(id='...', function=ChatCompletionOutputFunctionDefinition(
-                name='...', arguments='{"key": "value"}'), type='function')]
-        """
+        """Parse HuggingFace tool_calls into framework FunctionCallContent."""
         contents: list[FunctionCallContent] = []
 
         if not hasattr(message, "tool_calls") or not message.tool_calls:
@@ -299,6 +363,8 @@ class HuggingFaceChatClient(BaseChatClient):  # type: ignore[misc]
                 if contents:
                     yield ChatResponseUpdate(
                         contents=contents,
+                        role=Role.ASSISTANT,
+                        finish_reason=FinishReason.TOOL_CALLS,
                     )
 
         except Exception as e:

src/orchestrators/advanced.py

@@ -337,32 +337,52 @@ The final output should be a structured research report."""
         """
         Defensively extract text from a message object.
 
-        Fixes bug where message.text might return the object itself or its repr.
+        Handles ChatMessage objects from both OpenAI and HuggingFace clients.
+        ChatMessage has: .text (str), .contents (list of content objects)
+        Also handles plain string messages (e.g., WorkflowOutputEvent.data).
         """
         if not message:
             return ""
 
-        # Priority 1: .content (often the raw string or list of content)
+        # Priority 0: Handle plain string messages (e.g., WorkflowOutputEvent.data)
+        if isinstance(message, str):
+            # Filter out obvious repr-style noise
+            if not (message.startswith("<") and "object at" in message):
+                return message
+            return ""
+
+        # Priority 1: .text (standard ChatMessage text content)
+        if hasattr(message, "text") and message.text:
+            text = message.text
+            # Verify it's actually a string, not the object itself
+            if isinstance(text, str) and not (text.startswith("<") and "object at" in text):
+                return text
+
+        # Priority 2: .contents (list of FunctionCallContent, TextContent, etc.)
+        # This handles tool call responses from HuggingFace
+        if hasattr(message, "contents") and message.contents:
+            parts = []
+            for content in message.contents:
+                # TextContent has .text
+                if hasattr(content, "text") and content.text:
+                    parts.append(str(content.text))
+                # FunctionCallContent has .name and .arguments
+                elif hasattr(content, "name"):
+                    parts.append(f"[Tool: {content.name}]")
+            if parts:
+                return " ".join(parts)
+
+        # Priority 3: .content (legacy - some frameworks use singular)
         if hasattr(message, "content") and message.content:
             content = message.content
-            # If it's a list (e.g., Multi-modal), join text parts
+            if isinstance(content, str):
+                return content
             if isinstance(content, list):
                 return " ".join([str(c.text) for c in content if hasattr(c, "text")])
-            return str(content)
-
-        # Priority 2: .text (standard, but sometimes buggy/missing)
-        if hasattr(message, "text") and message.text:
-            # Verify it's not the object itself or a repr string
-            text = str(message.text)
-            if text.startswith("<") and "object at" in text:
-                # Likely a repr string, ignore if possible
-                pass
-            else:
-                return text
 
-        # Fallback: If we can't find clean text, return str(message)
-        # taking care to avoid infinite recursion if str() calls .text
-        return str(message)
+        # Fallback: Return empty string instead of repr
+        # The repr is useless for display purposes
+        return ""
 
     def _get_event_type_for_agent(self, agent_name: str) -> str:
         """Map agent name to appropriate event type.
@@ -456,9 +476,11 @@ The final output should be a structured research report."""
 
         elif isinstance(event, WorkflowOutputEvent):
             if event.data:
+                # Use _extract_text to properly handle ChatMessage objects
+                text = self._extract_text(event.data)
                 return AgentEvent(
                     type="complete",
-                    message=str(event.data),
+                    message=text if text else "Research complete (no synthesis)",
                     iteration=iteration,
                 )
 

tests/unit/clients/test_chat_client_factory.py

@@ -154,10 +154,10 @@ class TestHuggingFaceChatClient:
 
             client = HuggingFaceChatClient()
 
-            # Create mock messages
+            # Create mock messages (include contents=None for tool call processing)
             messages = [
-                MagicMock(spec=ChatMessage, role="user", text="Hello"),
-                MagicMock(spec=ChatMessage, role="assistant", text="Hi there!"),
+                MagicMock(spec=ChatMessage, role="user", text="Hello", contents=None),
+                MagicMock(spec=ChatMessage, role="assistant", text="Hi there!", contents=None),
             ]
 
             result = client._convert_messages(messages)
@@ -189,6 +189,7 @@ class TestHuggingFaceChatClient:
             mock_msg = MagicMock(spec=ChatMessage)
             mock_msg.role = Role.USER  # Enum, not string
             mock_msg.text = "Hello"
+            mock_msg.contents = None  # Required for tool call processing
 
             result = client._convert_messages([mock_msg])