Fix multi-turn tool calling in AppleIntelligenceChatClient, add chat overlay, and upgrade Agent Framework to rc2#34124
Open
mattleibow wants to merge 46 commits intomainfrom
Open
Fix multi-turn tool calling in AppleIntelligenceChatClient, add chat overlay, and upgrade Agent Framework to rc2#34124mattleibow wants to merge 46 commits intomainfrom
mattleibow wants to merge 46 commits intomainfrom
Conversation
Contributor
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. |
Comment on lines
12
to
21
| 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).
Suggested change
| 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; | |
| } | |
| } |
Comment on lines
+39
to
+43
| 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.
Comment on lines
208
to
212
| 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).
Comment on lines
+167
to
+170
| 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).
Suggested change
| $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).
Suggested change
| 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 |
Comment on lines
182
to
186
| 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.
Comment on lines
455
to
458
| $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.
Comment on lines
426
to
429
| $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).
Suggested change
| $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 |
Comment on lines
455
to
458
| $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).
mattleibow
force-pushed
the
dev/fix-apple-tools
branch
from
1444b8b to
cabef52
Compare
mattleibow
force-pushed
the
dev/fix-apple-tools
branch
from
f7866e8 to
d348317
Compare
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>
- 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>
Extract onUpdate logic into static local functions ProcessStreamUpdate and CompleteStream to reduce nesting. Wrap both callbacks in try/catch routing exceptions to channel.Writer.TryComplete(ex) to prevent unhandled managed exceptions from crossing the native Swift boundary. Change all channel.Writer.Complete() calls to TryComplete() so that if onUpdate already completed the channel with an error, the subsequent onComplete callback does not throw ChannelClosedException. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ToNative enumerates messages twice: once for callIdToName lookup and once for conversion. If a caller passes a forward-only IEnumerable, the second enumeration yields nothing. Materialize with .ToList() at method entry, then Prepend the system instruction afterward. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When a ToolCall update arrives during streaming, call chunker.Flush() before chunker.Reset() to emit any buffered content. For JsonStreamChunker, Reset() silently discards pending strings, containers, and open structures that Flush() would have emitted. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- StreamingResponseHandler: encapsulates channel, chunker, ProcessUpdate, Complete - NonStreamingResponseHandler: encapsulates TCS, Complete, CompleteCancelled - StreamUpdate record struct decouples handler from native ResponseUpdateNative - 16 new unit tests (10 streaming, 6 non-streaming) covering: - Content deltas, plain text chunking, tool call emit, tool result role - Flush before tool call, malformed JSON error, double completion safety - Post-tool fresh stream, empty text filtering - Valid/null response, error surfacing, cancellation, multiple messages - InternalsVisibleTo for device test project - All 137 tests pass (135 passed, 2 ignored) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Split ProcessUpdate into ProcessContent, ProcessToolCall, ProcessToolResult - Remove StreamUpdate record struct; call per-type methods directly - Move StreamingResponseHandler from MaciOS/ to Platform/ (no native deps) - Move 12 streaming tests from device tests to unit tests (file-per-concern) - Native enum dispatch stays in AppleIntelligenceChatClient onUpdate callback - All 12 unit tests + 126 device tests pass Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ChatBubble is a view model (INotifyPropertyChanged, ICommand) not a model. Move to ViewModels namespace and remove the exclusion from unit test csproj. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Decouple from native types: Complete(ChatResponse) instead of Complete(ChatResponseNative?) - Native conversion stays in AppleIntelligenceChatClient callback - FromNativeChatResponse reverted to private - Move 6 tests from device tests to unit tests (file-per-concern) - All 18 handler unit tests + 120 device tests pass Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rename ChatBubble to ChatBubbleViewModel with ObservableObject base - Use [ObservableProperty] and [RelayCommand] instead of manual INPC - Revert Swift prompt logic: last message is always the prompt, not the last user message. Callers handle their own message ordering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Split toTranscriptEntries into toUserEntry, toAssistantEntries, toSystemEntry, toToolEntries. Each handles one role in isolation. Ordering logic in assistant case is correct (preserves interleaving). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pass the chunker instance instead of a bool flag, removing the chunker selection concern from the handler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…vements, schema simplification, Swift error handling - Cancel native stream when onUpdate processing throws (prevents wasted CPU/battery) - Add default case to switch for unknown update types - Tighten test assertion to exact count (PlainTextStreamChunker is deterministic) - Add first-wins verification to DoubleComplete test - Simplify schema serialization with GetRawText() (remove intermediate allocation) - Replace silent compactMap with explicit errors in Swift converters Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tionalOnly Replace the 231-line NonFunctionInvokingChatClient wrapper with the built-in FunctionCallContent.InformationalOnly property from M.E.AI 10.3.0. When set to true, FunctionInvokingChatClient skips invocation of these function calls. Changes: - Set InformationalOnly=true on FunctionCallContent in StreamingResponseHandler and AppleIntelligenceChatClient.FromNative - Remove NonFunctionInvokingChatClient from sample app pipeline - Delete NonFunctionInvokingChatClient.cs (231 lines) and its tests (281 lines) - Add ToolCallLoggingTests verifying LoggingChatClient logs tool calls at Trace - Add InformationalOnly assertion to streaming handler unit test - Simplify FICC device test pipeline (remove WrapperClient/UnwrapperClient) - Add device test verifying InformationalOnly prevents FICC double-invocation - Update M.E.AI packages from 10.0.1 to 10.3.0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Each log level now has its own dedicated test that asserts what IS logged, not just what isn't: - Trace: full serialized content (function names, args, results) - Debug: 'invoked' and 'completed' lifecycle messages (no content) - Information: nothing (LoggingChatClient only uses Debug+Trace) - Streaming variants for Trace and Debug levels Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use realistic pipeline (MockClient → FICC → LoggingChatClient) built with
ChatClientBuilder. Two test files cover both tool calling patterns:
- InformationalOnlyToolCallLoggingTests: FICC skips invocation, LoggingChatClient
serializes tool content at Trace (our Apple Intelligence pattern)
- InvocableToolCallLoggingTests: FICC invokes tools and logs 'Invoking {name}'
at Trace and 'invocation completed. Duration:' at Debug
Shared helpers (LogCollector, MockToolCallClient, BuildPipeline) extracted to
ToolCallLoggingHelpers.cs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace manual .Use(inner => new LoggingChatClient(inner, logs)) with .UseLogging(loggerFactory) in all ToolCallLogging test pipelines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Since InformationalOnly tools are skipped by FunctionInvokingChatClient, we need to log them directly in the Apple client. Adds: - ILoggerFactory constructor parameter for AppleIntelligenceChatClient - LoggerMessage-based logging for tool calls and results: - Debug: Tool name and call ID (no sensitive data) - Trace: Full arguments and results (sensitive) - Logging in both non-streaming (FromNative) and streaming paths - 5 new device tests verifying Debug/Trace/Information levels, streaming, and no-tool scenarios - Updated sample app to pass ILoggerFactory to the client Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of logging when receiving tool call/result content from native
responses, log around the actual function invocation in AIFunctionToolAdapter
matching FunctionInvokingChatClient's pattern:
- Before: 'Invoking {Name}.' (Debug) / 'Invoking {Name}({Arguments}).' (Trace)
- After: '{Name} invocation completed. Duration: {Duration}' (Debug)
'{Name} invocation completed. Duration: {Duration}. Result: {Result}' (Trace)
- Error: '{Name} invocation failed.' (Error) / '{Name} invocation canceled.' (Debug)
Removed old 'Received tool call/result' logging from response conversion
and streaming callback paths.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DI resolves ILoggerFactory automatically via constructor injection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- 6 new tests: MultipleTools, StreamingTrace, InvocationOrder, ToolFailure, NoLoggerFactory safety, InformationSuppression - Handle concurrent tool invocation by native framework - Handle tool error propagation as NSErrorException Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mattleibow
force-pushed
the
dev/fix-apple-tools
branch
from
643f342 to
77226b9
Compare
- Fix thread-safety bug in DeviceTestLogCollector: add lock around List<T>.Add to prevent lost entries from concurrent tool invocations - Replace loose 'Contains' assertions with exact count, exact message, and exact log level checks for every scenario - MultipleTools test now correctly expects 4 entries (was 3 due to race condition in non-thread-safe List<T>) - Every test asserts exact entry count, log level per entry, and message format (StartsWith/Equals, not loose Contains) - Tests verify what SHOULD happen, not what might sorta happen Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move AppleIntelligenceChatClientToolCallLoggingTests into its own file next to the other Apple Intelligence test files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The upgrade from Microsoft.Agents.AI.Workflows 1.0.0-preview.251204.1 to 1.0.0-rc1 introduced AutoSendTurnToken on ChatProtocolExecutorOptions, defaulting to false. In the preview, ChatProtocolExecutor always forwarded the TurnToken after TakeTurnAsync; in rc1 it only does so when AutoSendTurnToken is true. Without this, TravelPlannerExecutor completes but the workflow engine never triggers ResearcherExecutor because no TurnToken is forwarded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JsonSchemaDecoder.toDynamicSchema and parseJsonProperty now throw descriptive errors (SchemaError) instead of silently returning nil. This surfaces unsupported types, missing properties, and missing array items at parse time rather than silently dropping schema properties. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rExecutor Upgrade Microsoft.Agents.AI packages from rc1 to rc2 and fix a workflow stall where the itinerary pipeline would stop after the first executor. Changes: - Upgrade Microsoft.Agents.AI to 1.0.0-rc2 - Upgrade Microsoft.Agents.AI.Workflows to 1.0.0-rc2 - Upgrade Microsoft.Agents.AI.Hosting to 1.0.0-preview.260225.1 - Add nuget.org package source for rc2 packages - Fix TravelPlannerExecutor workflow stall (details below) Root cause: ChatProtocolExecutor sets AutoSendMessageHandlerResultObject to false and only declares List<ChatMessage> and TurnToken in its protocol. When TravelPlannerExecutor sent a TravelPlanResult via context.SendMessageAsync(), the edge router could not map the type through SendTypeTranslator, causing the message to be silently dropped with DroppedTypeMismatch. Executor 2 never received it and the workflow stalled. Fix: 1. Override ConfigureProtocol to add SendsMessage<TravelPlanResult>() so the edge router recognizes the custom type 2. Set AutoSendTurnToken = false since downstream executors are typed (Executor<TravelPlanResult>) and do not handle TurnToken Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mattleibow
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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!
Summary
This PR fixes multi-turn tool calling in
AppleIntelligenceChatClient, adds a chat overlay UI to the AI sample app, and upgrades the Microsoft Agent Framework from rc1 to rc2.Changes
1. Fix multi-turn tool calling in AppleIntelligenceChatClient
When a conversation included tool calls, the next user message would fail with
"The content type '...' is not supported by Apple Intelligence chat APIs"becauseFunctionCallContentandFunctionResultContentfrom previous turns were passed back to the native Apple Intelligence API, which only supports text content.Fix: Filter conversation history to only include content types supported by the Apple Intelligence API before sending to the native layer. Tool call/result content is used internally for the Microsoft.Extensions.AI pipeline but stripped before reaching Apple's API.
Refactoring for testability:
StreamingResponseHandler— manages channel, chunker, and update processing for streaming responsesNonStreamingResponseHandler— manages TaskCompletionSource for non-streaming responses2. Tool call logging (FICC-style)
Replace the custom
NonFunctionInvokingChatClientwrapper with the built-inFunctionCallContent.InformationalOnlypattern from Microsoft.Extensions.AI. Tool calls from Apple Intelligence are logged as informational-onlyFunctionCallContententries in the conversation history, making them visible to the caller without requiring a separate wrapper class.Tests:
InformationalOnlyToolCallLoggingTests— verifies informational-only tool calls are logged correctlyInvocableToolCallLoggingTests— verifies invocable tool calls produce both call and result contentAppleIntelligenceChatClientToolCallLoggingTests— device tests for on-device tool call logging3. Chat overlay UI in sample app
Add a reusable chat overlay with markdown rendering for the AI sample app:
ChatOverlayView— XAML overlay with message bubbles, typing indicatorsChatViewModel/ChatBubbleViewModel— MVVM view modelsChatService— service layer connecting to the AI agentChatBubbleTemplateSelector— user vs. assistant bubble stylingMarkdownConverter— converts markdown to MAUI formatted textLandmarksPageandTripPlanningPage4. Multi-turn function calling device tests
New comprehensive device test suite (
ChatClientFunctionCallingTests) covering:5. Upgrade Microsoft Agent Framework to rc2
Microsoft.Agents.AI: 1.0.0-rc1 → 1.0.0-rc2Microsoft.Agents.AI.Workflows: 1.0.0-rc1 → 1.0.0-rc2Microsoft.Agents.AI.Hosting: 1.0.0-preview.260219.1 → 1.0.0-preview.260225.1Fix workflow stall in TravelPlannerExecutor: In rc2,
ChatProtocolExecutorsetsAutoSendMessageHandlerResultObject = falseand only declaresList<ChatMessage>andTurnTokenin its protocol. Custom types sent viacontext.SendMessageAsync()must be explicitly declared by overridingConfigureProtocol. Without this,TravelPlanResultwas silently dropped by the edge router (DroppedTypeMismatch) and executor 2 never received it.ConfigureProtocolto addSendsMessage<TravelPlanResult>()AutoSendTurnToken = falsesince downstream executors are typed and don't handleTurnToken6. Swift native layer improvements
JsonSchemaDecoderinstead ofcompactMap(surfaces errors properly)ChatClient.swifttranscript conversion with per-role functions7. Stream chunker improvements
Reset()method toJsonStreamChunker,PlainTextStreamChunker, andStreamChunkerBaseTest coverage