Fix multi-turn tool calling in AppleIntelligenceChatClient and add AI sample chat overlay#34124
Fix multi-turn tool calling in AppleIntelligenceChatClient and add AI sample chat overlay#34124mattleibow wants to merge 19 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Apple Intelligence multi-turn tool-calling transcript/streaming issues and adds an AI chat overlay (with tool calling) to the Essentials.AI sample app, plus small test/dev tooling updates.
Changes:
- Add multi-turn tool-calling device tests and update Apple Intelligence native/managed interop to better handle tool-call/tool-result content.
- Reset streaming chunker state across tool boundaries to avoid dropped post-tool text.
- Introduce a sample chat overlay UI + view model/service wiring, and extend repo scripts/skills to run AI device tests and parameterize Sandbox runs.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/AI/tests/Essentials.AI.DeviceTests/Tests/ChatClientFunctionCallingTests.cs | Adds new multi-turn tool-calling device tests. |
| src/AI/src/Essentials.AI/Platform/StreamChunkerBase.cs | Adds Reset() contract for stream chunkers. |
| src/AI/src/Essentials.AI/Platform/PlainTextStreamChunker.cs | Implements Reset() for plain-text delta chunking. |
| src/AI/src/Essentials.AI/Platform/MaciOS/AppleIntelligenceChatClient.cs | Resets chunker on tool calls; extends content conversion for function call/result history. |
| src/AI/src/Essentials.AI/Platform/JsonStreamChunker.cs | Implements Reset() for JSON delta chunking state. |
| src/AI/src/AppleNative/EssentialsAI/ChatMessageContent.swift | Extends native function-result content with a name property. |
| src/AI/src/AppleNative/EssentialsAI/ChatClient.swift | Fixes prompt selection for tool-loop scenarios; builds transcript entries for tool calls/outputs. |
| src/AI/src/AppleNative/ApiDefinitions.cs | Updates binding for FunctionResultContentNative name and designated initializer signature. |
| src/AI/samples/Essentials.AI.Sample/Views/ChatOverlayView.xaml.cs | Adds overlay show/hide + auto-scroll behavior. |
| src/AI/samples/Essentials.AI.Sample/Views/ChatOverlayView.xaml | Defines overlay UI and chat bubble templates. |
| src/AI/samples/Essentials.AI.Sample/Views/ChatBubbleTemplateSelector.cs | Adds template selector for user/assistant/tool bubbles. |
| src/AI/samples/Essentials.AI.Sample/ViewModels/ChatViewModel.cs | Adds streaming chat view model with tool-call/result bubble handling. |
| src/AI/samples/Essentials.AI.Sample/Services/ChatService.cs | Adds tool-backed chat service (landmarks/weather/tags/language/plan-trip). |
| src/AI/samples/Essentials.AI.Sample/Pages/TripPlanningPage.xaml.cs | Adds FAB + overlay wiring on TripPlanningPage. |
| src/AI/samples/Essentials.AI.Sample/Pages/TripPlanningPage.xaml | Adds FAB button to TripPlanningPage layout. |
| src/AI/samples/Essentials.AI.Sample/Pages/LandmarksPage.xaml.cs | Adds FAB + overlay wiring and “plan trip” navigation hook. |
| src/AI/samples/Essentials.AI.Sample/Pages/LandmarksPage.xaml | Adds FAB button to LandmarksPage layout. |
| src/AI/samples/Essentials.AI.Sample/Models/ChatBubble.cs | Adds chat bubble model + expand/collapse command. |
| src/AI/samples/Essentials.AI.Sample/MauiProgram.cs | Registers ChatViewModel and ChatService in DI. |
| .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 | Adds AI device test project and improves artifact path derivation. |
| .github/skills/run-device-tests/SKILL.md | Documents AI as a valid device test project target. |
| .github/scripts/BuildAndRunSandbox.ps1 | Parameterizes project/bundle/app/test-dir and updates artifact/log handling. |
| InitializeComponent(); | ||
| } | ||
|
|
||
| public void Initialize(ChatViewModel viewModel) | ||
| { | ||
| _viewModel = viewModel; | ||
| BindingContext = viewModel; | ||
| viewModel.Messages.CollectionChanged += OnMessagesChanged; | ||
| } | ||
|
|
There was a problem hiding this comment.
Initialize subscribes to viewModel.Messages.CollectionChanged but never unsubscribes. Since ChatViewModel is registered as a singleton, this handler can keep the view/page alive after navigation and cause a leak. Detach the handler when the view is unloaded/removed (or before re-initializing).
| InitializeComponent(); | |
| } | |
| public void Initialize(ChatViewModel viewModel) | |
| { | |
| _viewModel = viewModel; | |
| BindingContext = viewModel; | |
| viewModel.Messages.CollectionChanged += OnMessagesChanged; | |
| } | |
| InitializeComponent(); | |
| Unloaded += OnUnloaded; | |
| } | |
| public void Initialize(ChatViewModel viewModel) | |
| { | |
| if (_viewModel?.Messages is not null) | |
| { | |
| _viewModel.Messages.CollectionChanged -= OnMessagesChanged; | |
| } | |
| _viewModel = viewModel; | |
| BindingContext = viewModel; | |
| viewModel.Messages.CollectionChanged += OnMessagesChanged; | |
| } | |
| void OnUnloaded(object? sender, EventArgs e) | |
| { | |
| if (_viewModel?.Messages is not null) | |
| { | |
| _viewModel.Messages.CollectionChanged -= OnMessagesChanged; | |
| } | |
| } |
| private async void OnNavigateToTrip(Landmark landmark) | ||
| { | ||
| // Close chat overlay first if open | ||
| await _chatOverlay.Hide(); | ||
|
|
There was a problem hiding this comment.
OnNavigateToTrip can be invoked from a tool callback on a background thread (AppleIntelligenceChatClient tool adapter executes tools off the UI thread), but this method runs UI code (animations + Shell navigation). Dispatch to the UI thread (MainThread/Dispatcher) before calling Hide()/GoToAsync.
| if (landmark is null) | ||
| return $"Landmark '{landmarkName}' not found. Try searching with search_landmarks first."; | ||
|
|
||
| NavigateToTripRequested?.Invoke(landmark); | ||
| return $"Navigating to trip planner for {landmark.Name}! A multi-day itinerary will be generated for you."; |
There was a problem hiding this comment.
NavigateToTripRequested is invoked directly from the tool method. Tool invocations run off the UI thread, so UI subscribers may be called from a background thread and crash when navigating/updating UI. Consider raising this event via the UI dispatcher (or otherwise marshalling to the main thread).
| catch (OperationCanceledException) | ||
| { | ||
| // User cancelled | ||
| } |
There was a problem hiding this comment.
If streaming is cancelled, the OperationCanceledException path doesn’t clear any in-progress assistant bubble state, so the UI can be left showing IsStreaming=true indefinitely. Consider ensuring assistantBubble.IsStreaming is set to false (on the UI thread) in finally even when the enumeration ends via cancellation.
|
|
||
| # Get app process ID for later cleanup | ||
| $catalystAppProcess = Get-Process -Name "Maui.Controls.Sample.Sandbox" -ErrorAction SilentlyContinue | Select-Object -First 1 | ||
| $catalystAppProcess = Get-Process -Name $projectBaseName -ErrorAction SilentlyContinue | Select-Object -First 1 |
There was a problem hiding this comment.
MacCatalyst process discovery uses $projectBaseName, but the script supports overriding -AppName and uses it for the .app path. If AppName differs, Get-Process may not find the running app for cleanup. Prefer using $AppName (or fall back to $projectBaseName only when AppName is not set).
| $catalystAppProcess = Get-Process -Name $projectBaseName -ErrorAction SilentlyContinue | Select-Object -First 1 | |
| $catalystProcessName = if ($AppName) { $AppName } else { $projectBaseName } | |
| $catalystAppProcess = Get-Process -Name $catalystProcessName -ErrorAction SilentlyContinue | Select-Object -First 1 |
|
|
||
| FunctionResultContent functionResult => [new FunctionResultContentNative( | ||
| functionResult.CallId ?? string.Empty, | ||
| string.Empty, // FunctionResultContent doesn't carry the tool name; the callId links to the ToolCall |
There was a problem hiding this comment.
FunctionResultContentNative now includes a Name and Swift uses it for Transcript.ToolOutput(toolName:), but managed->native conversion hardcodes the name to string.Empty. This can prevent tool outputs from being correctly reconstructed on follow-up turns. Populate the name when possible (or infer it by mapping callId -> FunctionCallContent.Name from the surrounding message history).
| string.Empty, // FunctionResultContent doesn't carry the tool name; the callId links to the ToolCall | |
| functionResult.Name ?? string.Empty, // Use the function name when available; callId still links to the ToolCall |
| finally | ||
| { | ||
| IsSending = false; | ||
| _cts = null; | ||
| } |
There was a problem hiding this comment.
CancellationTokenSource created per send is never disposed. Dispose the CTS in finally (e.g., capture it to a local variable and dispose it after the loop) to avoid leaking registrations/resources across multiple sends.
| $logStartTime = (Get-Date).AddMinutes(-2).ToString("yyyy-MM-dd HH:mm:ss") | ||
| $catalystLogCommand = "log show --level debug --predicate 'process contains `"Maui.Controls.Sample.Sandbox`" OR processImagePath contains `"Maui.Controls.Sample.Sandbox`"' --start `"$logStartTime`" --style compact" | ||
| $processFilter = [System.IO.Path]::GetFileNameWithoutExtension($SandboxProject) | ||
| $catalystLogCommand = "log show --level debug --predicate 'process contains `"$processFilter`" OR processImagePath contains `"$processFilter`"' --start `"$logStartTime`" --style compact" | ||
| Invoke-Expression "$catalystLogCommand > `"$deviceLogFile`" 2>&1" |
There was a problem hiding this comment.
MacCatalyst os_log fallback also filters by $SandboxProject base name; with -AppName override this may not match the actual process name/image path. Use the same filter value as the app launch name ($AppName) for consistency.
| $iosLogCommand = "xcrun simctl spawn booted log show --predicate 'processImagePath contains `"$processFilter`"' --start `"$logStartTime`" --style compact" | ||
|
|
||
| Write-Info "Capturing logs from last 2 minutes..." | ||
| Invoke-Expression "$iosLogCommand > `"$deviceLogFile`" 2>&1" |
There was a problem hiding this comment.
The construction of the iosLogCommand string with processFilter and subsequent use of Invoke-Expression introduces a command injection risk. Because processFilter is derived from GetFileNameWithoutExtension($SandboxProject), an attacker who can influence -ProjectPath (e.g., by including single quotes or shell metacharacters in the project file name) can break out of the quoted predicate and inject arbitrary PowerShell/CLI arguments when the command string is evaluated. To mitigate this, avoid Invoke-Expression for this path and pass arguments to xcrun simctl as a properly parameterized call (or rigorously escape/sanitize processFilter so that it cannot modify the command structure).
| $iosLogCommand = "xcrun simctl spawn booted log show --predicate 'processImagePath contains `"$processFilter`"' --start `"$logStartTime`" --style compact" | |
| Write-Info "Capturing logs from last 2 minutes..." | |
| Invoke-Expression "$iosLogCommand > `"$deviceLogFile`" 2>&1" | |
| # Escape any double quotes in the process name to keep the predicate well-formed | |
| $escapedProcessFilter = $processFilter -replace '"', '\"' | |
| $predicate = "processImagePath contains `"$escapedProcessFilter`"" | |
| Write-Info "Capturing logs from last 2 minutes..." | |
| & xcrun simctl spawn booted log show --predicate $predicate --start $logStartTime --style compact > $deviceLogFile 2>&1 |
| $logStartTime = (Get-Date).AddMinutes(-2).ToString("yyyy-MM-dd HH:mm:ss") | ||
| $catalystLogCommand = "log show --level debug --predicate 'process contains `"Maui.Controls.Sample.Sandbox`" OR processImagePath contains `"Maui.Controls.Sample.Sandbox`"' --start `"$logStartTime`" --style compact" | ||
| $processFilter = [System.IO.Path]::GetFileNameWithoutExtension($SandboxProject) | ||
| $catalystLogCommand = "log show --level debug --predicate 'process contains `"$processFilter`" OR processImagePath contains `"$processFilter`"' --start `"$logStartTime`" --style compact" | ||
| Invoke-Expression "$catalystLogCommand > `"$deviceLogFile`" 2>&1" |
There was a problem hiding this comment.
The construction of catalystLogCommand with processFilter and execution via Invoke-Expression creates a similar command injection vector. Since processFilter comes from GetFileNameWithoutExtension($SandboxProject), a maliciously crafted project file name (containing quotes or other metacharacters) can break out of the intended quoted predicate and inject additional commands or arguments when this string is evaluated. To harden this, avoid Invoke-Expression here and instead invoke log show with explicit arguments (or robustly escape/sanitize processFilter so that it cannot alter the command syntax).
Fix two issues that caused multi-turn conversations with tool calling to fail: 1. C# side: Add FunctionCallContent/FunctionResultContent to native conversion and filter empty messages from conversation history. 2. Swift side: Change prepareSession to find the last User message as role message after FunctionInvokingChatClient processes tool results. Also add Name property to FunctionResultContentNative binding and support ToolCall/ToolCalls/ToolOutput transcript entries in Swift. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add ChatClientFunctionCallingTestsBase with three tests: - GetResponseAsync_WithToolCall_ReturnsToolAndTextContent - GetResponseAsync_MultiTurnWithTools_HandlesConversationHistory - GetStreamingResponseAsync_WithToolCall_StreamsToolAndText Tests verify that function call/result content flows correctly through the Apple Intelligence chat client and that follow-up messages after tool execution succeed without errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apple Intelligence's FoundationModels resets the cumulative text stream after native tool execution. The PlainTextStreamChunker was computing deltas assuming monotonic growth, which caused the first N characters of post-tool text to be silently dropped (where N = length of pre-tool text). For example, 'Here are some...' lost 'Here' because the chunker treated it as overlapping with the pre-tool 'null' output. Add Reset() to StreamChunkerBase and call it when a ToolCall update arrives, so post-tool text is treated as a fresh stream. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a floating action button (FAB) chat interface to the AI sample app with a streaming chat overlay that supports tool calling. Includes: - ChatBubble model with 4 types (User/Assistant/ToolCall/ToolResult) - ChatViewModel with streaming, tool deduplication, and junk bubble cleanup - ChatOverlayView with collapsible tool bubbles and animations - ChatService with 8 tools: search_landmarks, list_landmarks_by_continent, get_landmark_details, search_points_of_interest, get_weather, generate_tags, set_language, plan_trip - FAB buttons on LandmarksPage and TripPlanningPage - Navigation event for plan_trip tool Tools use Apple Intelligence native tool calling (not FunctionInvokingChatClient) since FoundationModels handles the tool loop directly via ToolNative/ToolCallWatcher. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add AI project support to Run-DeviceTests.ps1 and update SKILL.md - Fix artifact path bug in Run-DeviceTests.ps1 for AI project - Parameterize BuildAndRunSandbox.ps1 with -ProjectPath, -BundleId, -AppName, -TestDir to support any MAUI sample app (defaults to Sandbox app for backward compatibility) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- 8 tests for PlainTextStreamChunker.Reset() including tool call boundary simulation, multiple resets, idempotency, and multi-segment concatenation - 7 tests for JsonStreamChunker.Reset() including fresh stream after reset, re-emission of cleared strings, and mid-string reset safety - Fix UnitTests.csproj to exclude ChatBubble.cs from Models/** glob (uses MAUI Command type not available in net10.0 test project) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add New Chat (🗑) button in header to clear conversation - Add NewChatCommand to ChatViewModel (cancels pending, clears messages/history) - Responsive panel: 90% width/height with max 400w x 700h for desktop - Panel now HorizontalOptions=Center so it doesn't stretch full width - Dynamic TranslationY for slide animation based on actual panel height - Replace hardcoded 500px height with computed size via SizeChanged Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Changed chat panel HorizontalOptions to End for right alignment - Added 'Thinking...' bubble immediately after user sends message - Fixed post-tool-call text: create new bubble if thinking bubble was removed - Validated with Appium: Africa landmarks query works end-to-end Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace SizeChanged width calculation with XAML MaximumWidthRequest=500 - Keep minimal SizeChanged for height only (MAUI End-aligned elements overflow) - Round all corners for floating bubble appearance - Add MarkdownConverter for assistant bubbles (bold, italic, code spans) - Panel: 500w x 800h max, responsive on small windows Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1444b8b to
cabef52
Compare
- Replace regex-based markdown converter with Markdig AST walker - Supports bold, italic, code, lists, headings, links, code blocks - Unsupported elements fall back to plain text Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add today's date to system prompt instead of separate GetCurrentDate tool - Change weather date param from string to DateTimeOffset for typed schema - Remove ResolveDate string parsing — AI computes dates from system prompt - Fix responsive panel sizing with SizeChanged (MAUI centers MaxWidth-capped elements) - Validate date range (today through +7 days) with clear error messages Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add FunctionCallContent and FunctionResultContent to conversation history so the Apple Intelligence transcript includes the complete tool interaction chain on subsequent turns. Previously only TextContent was kept, causing the model to lose context about which tools were called and what they returned. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Test the exact pattern ChatViewModel uses: collect streaming content, build history by role (Assistant→FunctionCallContent, Tool→FunctionResultContent, Assistant→TextContent), then send follow-up. Verifies the model can reference specific tool results (47°F) from prior turns. Both tests pass on MacCatalyst: 112 total, 110 passed, 0 failed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of collecting all content into a temp list and grouping by type (which reorders call→result→call→result into call→call→result→result), add each item to _conversationHistory as it streams in. This preserves the natural interleaved order of tool call/result pairs. Removes allContents temp list and post-stream processing block. Same fix applied to device tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix Apple Intelligence 'null' text sentinel: Filter at Swift layer instead of C# — guard in streamResponse loop skips empty/null content before tool calls. Removed all 'null' text string filters from C# and ViewModel. - Fix FunctionResultContent tool name: Build callId→name lookup from FunctionCallContent so FunctionResultContent passes correct tool name to Swift's Transcript.ToolOutput. - Fix thread safety: PlanTripAsync dispatches NavigateToTripRequested to main thread via MainThread.BeginInvokeOnMainThread. - Fix timezone: Use date.DateTime instead of date.LocalDateTime to prevent DateTimeOffset→DateOnly shifting to previous day in non-UTC timezones. - Fix MarkdownConverter bounds: Clamp Markdig Span.Start/Length to text length to prevent IndexOutOfRangeException. - Fix error message: Improved ArgumentException when all messages filtered. - Add ILogger<ChatViewModel> debug logging for conversation flow tracing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove ILogger<ChatViewModel> and IDispatcher from ChatViewModel. SendMessageAsync runs on UI thread from button click; await foreach preserves SynchronizationContext, making Dispatch() redundant. - Fix toTranscriptEntries: process assistant message contents in order, flushing batches when content type changes. Preserves interleaving instead of grouping all text first then all tool calls. - Inline toTranscriptSegment into toTranscriptEntries (single pass). - Add device test: no 'null' text leaks through streaming pipeline during tool-calling conversations. - Add device test: content order preserved (call→result pairs stay adjacent) through streaming history building. - All tests pass: 290 unit, 112 device (110 passed, 2 skipped). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removed text != "null" guards from both Swift streaming loops. Device test GetStreamingResponseAsync_WithToolCalling_NoNullTextContent confirms no 'null' text leaks through during tool-calling streams. Investigation with 5 AI models + empirical testing shows: - Text path: StreamResponse<String>.content is String, empty = "" - The original 'null' observation was likely caused by other bugs since fixed (chunker reset, history ordering, prompt extraction) - GPT-5.1-Codex: 'null' only occurs on schema path via .jsonString on empty GeneratedContent — not applicable to text streaming Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Always remove pre-tool assistant bubble when tool calls arrive instead of conditionally keeping it based on text content - Add ViewModelSimulation test that replicates ChatViewModel state machine through full middleware pipeline (wrap→FICC→unwrap) - Add raw client null text test (3 runs) for landmarks/Africa query - Add comprehensive stream capture test for null text detection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
Fixes multi-turn conversations with tool calling in
AppleIntelligenceChatClientand adds a chat overlay with 8 AI tools to the sample app.Bug Fixes
1. Multi-turn tool calling failure (C# + Swift)
ToNative(AIContent)only handledTextContent— added support forFunctionCallContentandFunctionResultContentChatClient.swift prepareSessionusedmessages.last!as prompt, which could be a Tool role message afterFunctionInvokingChatClientprocesses tool results — changed to find the last User message vialastIndex(where:)Nameproperty toFunctionResultContentNativebinding2. Stream chunker not resetting across tool boundaries
PlainTextStreamChunkercomputed deltas assuming monotonic text growth, but Apple Intelligence resets the text stream after native tool executionReset()toStreamChunkerBaseand call it whenToolCallupdates arriveNew Features
Chat overlay with 8 AI tools in the sample app:
search_landmarks,list_landmarks_by_continent,get_landmark_details,search_points_of_interest,get_weather,generate_tags,set_language,plan_tripFunctionInvokingChatClient) since FoundationModels handles the tool loop directlyInfrastructure
Run-DeviceTests.ps1skillBuildAndRunSandbox.ps1with-ProjectPath,-BundleId,-AppName,-TestDirTesting