[AI] Sample: Detail page, semantic search, streaming, and UI polish#34576
[AI] Sample: Detail page, semantic search, streaming, and UI polish#34576mattleibow merged 19 commits intomainfrom
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34576Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34576" |
There was a problem hiding this comment.
Pull request overview
This PR evolves the Essentials.AI.Sample app’s browsing → detail → planning flow by adding a new Landmark Detail page with AI-driven content, introducing a semantic-search abstraction + implementation, and updating UX/styling across the landmarks and itinerary experiences.
Changes:
- Added LandmarkDetailPage + LandmarkDetailViewModel and updated navigation flow from Landmarks → Detail → Trip planning.
- Introduced ISemanticSearchService and an in-memory EmbeddingSearchService, and refactored
DataServiceto index/search via the abstraction. - Updated UI/UX styling and interaction behavior across several Views/Pages (search UI, backgrounds, transparency/input handling).
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/AI/samples/Essentials.AI.Sample/Views/Landmarks/LandmarkListItemView.xaml | Makes list item background transparent and prevents child hit-testing so border tap works reliably. |
| src/AI/samples/Essentials.AI.Sample/Views/Landmarks/LandmarkHorizontalListView.xaml | Adds padding to horizontal landmarks list layout. |
| src/AI/samples/Essentials.AI.Sample/Views/Landmarks/LandmarkFeaturedItemView.xaml | Aligns featured item styling with list items (transparent bg, input transparency). |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/LandmarkTripView.xaml.cs | Deleted (trip view merged into TripPlanningPage). |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/LandmarkTripView.xaml | Deleted (trip view merged into TripPlanningPage). |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/LandmarkDescriptionView.xaml.cs | Deleted (replaced by LandmarkDetailPage). |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/LandmarkDescriptionView.xaml | Deleted (replaced by LandmarkDetailPage). |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/ItineraryView.xaml | Updates rationale card colors/background styling. |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/DayView.xaml | Switches Border to Background usage. |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/ActivityListView.xaml | Switches Border to Background usage. |
| src/AI/samples/Essentials.AI.Sample/Views/ChatOverlayView.xaml | Switches multiple Borders/Buttons to Background usage. |
| src/AI/samples/Essentials.AI.Sample/ViewModels/TripPlanningViewModel.cs | Removes initial state flow; auto-starts itinerary generation on load. |
| src/AI/samples/Essentials.AI.Sample/ViewModels/LandmarksViewModel.cs | Adds debounced search query support + recent searches tracking + dispatcher usage. |
| src/AI/samples/Essentials.AI.Sample/ViewModels/LandmarkDetailViewModel.cs | New VM powering the detail page: similar destinations, tags, AI travel tip, language preference. |
| src/AI/samples/Essentials.AI.Sample/Services/TaggingService.cs | Switches tag generation from structured JSON response to comma-separated parsing. |
| src/AI/samples/Essentials.AI.Sample/Services/ISemanticSearchService.cs | New semantic search abstraction + result record. |
| src/AI/samples/Essentials.AI.Sample/Services/EmbeddingSearchService.cs | New in-memory embedding search implementation (chunking + cosine similarity + keyword boost). |
| src/AI/samples/Essentials.AI.Sample/Services/DataService.cs | Refactors indexing/search to use ISemanticSearchService and readiness tasks. |
| src/AI/samples/Essentials.AI.Sample/Services/ChatService.cs | Uses dispatcher for navigation callback; updates constructor dependencies. |
| src/AI/samples/Essentials.AI.Sample/Resources/Styles/Styles.xaml | Removes global BoxView background style. |
| src/AI/samples/Essentials.AI.Sample/Resources/Styles/Colors.xaml | Adds Gray700. |
| src/AI/samples/Essentials.AI.Sample/Pages/TripPlanningPage.xaml.cs | Removes chat overlay wiring; triggers itinerary initialization on load. |
| src/AI/samples/Essentials.AI.Sample/Pages/TripPlanningPage.xaml | Inlines trip planning UI and updates layout for full-bleed header + back button. |
| src/AI/samples/Essentials.AI.Sample/Pages/LandmarksPage.xaml.cs | Adds iOS/MacCatalyst handler tweaks for search Entry; navigates to new detail page. |
| src/AI/samples/Essentials.AI.Sample/Pages/LandmarksPage.xaml | Adds search UI; hides featured section during search; style adjustments. |
| src/AI/samples/Essentials.AI.Sample/Pages/LandmarkDetailPage.xaml.cs | New page code-behind handling back, plan trip, and similar landmark navigation. |
| src/AI/samples/Essentials.AI.Sample/Pages/LandmarkDetailPage.xaml | New detail page UI: hero image, tags, AI tip, similar destinations, language picker, plan trip CTA. |
| src/AI/samples/Essentials.AI.Sample/Models/PointOfInterest.cs | Removes stored embeddings from model. |
| src/AI/samples/Essentials.AI.Sample/Models/Landmark.cs | Removes stored embeddings from model. |
| src/AI/samples/Essentials.AI.Sample/MauiProgram.cs | Registers new page/VM; registers ISemanticSearchService for iOS/MacCatalyst. |
| src/AI/samples/Essentials.AI.Sample/AppShell.xaml.cs | Registers new LandmarkDetailPage route. |
src/AI/samples/Essentials.AI.Sample/Pages/LandmarkDetailPage.xaml
Outdated
Show resolved
Hide resolved
src/AI/samples/Essentials.AI.Sample/Pages/LandmarkDetailPage.xaml
Outdated
Show resolved
Hide resolved
src/AI/samples/Essentials.AI.Sample/ViewModels/LandmarkDetailViewModel.cs
Show resolved
Hide resolved
mattleibow
changed the title
There was a problem hiding this comment.
Pull request overview
This PR significantly expands and refines the Essentials.AI.Sample app’s UX and navigation flow (Landmarks → new Detail page → Trip planning), while also enhancing streaming support in Essentials.AI with a passthrough mode and accompanying tests.
Changes:
- Added a new
LandmarkDetailPage+LandmarkDetailViewModelwith AI travel tips, tags, weather, similar-destination search, and language selection. - Introduced semantic search abstractions (
ISemanticSearchService) and an embedding-backed in-memory implementation, then refactoredDataServiceand UI to use it. - Updated
StreamingResponseHandlerto support “no chunker” passthrough streaming, plus new unit/device tests around incremental streaming behavior.
Reviewed changes
Copilot reviewed 42 out of 44 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/AI/src/Essentials.AI/Platform/StreamingResponseHandler.cs | Adds nullable chunker + passthrough streaming mode and adjusts flush/reset behavior accordingly. |
| src/AI/tests/Essentials.AI.UnitTests/Tests/StreamingResponseHandlerTests/Passthrough.cs | New unit tests validating passthrough (no chunker) streaming semantics. |
| src/AI/tests/Essentials.AI.DeviceTests/Tests/ChatClientStreamingTestsBase.cs | Adds device tests intended to validate incremental updates and streaming/non-streaming parity. |
| src/AI/samples/Essentials.AI.Sample/Pages/LandmarksPage.xaml | Adds a custom search entry UI; hides featured section during search; edge-to-edge layout tweaks. |
| src/AI/samples/Essentials.AI.Sample/Pages/LandmarksPage.xaml.cs | Adds platform handler tweaks to remove native borders/focus rings; routes taps to new detail page and passes recent searches. |
| src/AI/samples/Essentials.AI.Sample/ViewModels/LandmarksViewModel.cs | Adds debounced search query pipeline + recent-search tracking; switches UI thread dispatching to injected IDispatcher. |
| src/AI/samples/Essentials.AI.Sample/Pages/LandmarkDetailPage.xaml | New landmark detail UI with hero image, tags, AI tip panel, similar destinations, language picker, and bottom CTA bar. |
| src/AI/samples/Essentials.AI.Sample/Pages/LandmarkDetailPage.xaml.cs | New page code-behind: init/cancel wiring, plan-trip navigation, similar-destination navigation. |
| src/AI/samples/Essentials.AI.Sample/ViewModels/LandmarkDetailViewModel.cs | New VM: loads similar destinations, weather, tags, and AI travel tip concurrently with cancellation support. |
| src/AI/samples/Essentials.AI.Sample/Pages/TripPlanningPage.xaml | Reworks trip planning to match new edge-to-edge/gradient pattern; removes embedded old trip view. |
| src/AI/samples/Essentials.AI.Sample/Pages/TripPlanningPage.xaml.cs | Simplifies page (removes chat overlay hookup) and switches to fire-and-forget initialization. |
| src/AI/samples/Essentials.AI.Sample/ViewModels/TripPlanningViewModel.cs | Refactors trip planning state machine; removes initial/tags/button flow; auto-starts itinerary generation; adds query language input. |
| src/AI/samples/Essentials.AI.Sample/Services/DataService.cs | Refactors from “embedding generation” to ISemanticSearchService indexing + search; adds POI name dictionary. |
| src/AI/samples/Essentials.AI.Sample/Services/ISemanticSearchService.cs | New interface to abstract semantic indexing/search and readiness signaling. |
| src/AI/samples/Essentials.AI.Sample/Services/EmbeddingSearchService.cs | New in-memory embedding search using cosine similarity + hybrid keyword boost + sentence chunking. |
| src/AI/samples/Essentials.AI.Sample/MauiProgram.cs | Registers new page/vm; removes language preference service; registers semantic search service for iOS/MacCatalyst Apple embeddings. |
| src/AI/samples/Essentials.AI.Sample/AppShell.xaml.cs | Registers LandmarkDetailPage route for Shell navigation. |
| src/AI/samples/Essentials.AI.Sample/Services/ItineraryService.cs | Removes legacy landmark/dayCount overload (language preference removed). |
| src/AI/samples/Essentials.AI.Sample/Services/ChatService.cs | Removes “set language” tool; switches navigation dispatch to injected IDispatcher. |
| src/AI/samples/Essentials.AI.Sample/Services/TaggingService.cs | Switches tag generation from structured JSON to comma-separated text parsing for broader compatibility. |
| src/AI/samples/Essentials.AI.Sample/Services/PromptBasedSchemaClient.cs | New delegating client translating JSON schema requests into prompt instructions and stripping markdown fences. |
| src/AI/samples/Essentials.AI.Sample/Services/WeatherService.cs | Ensures invariant-culture formatting for lat/long query parameters. |
| src/AI/samples/Essentials.AI.Sample/AI/WorkflowModels.cs | Updates workflow DTOs to use shorter JSON property names (via JsonPropertyName). |
| src/AI/samples/Essentials.AI.Sample/AI/ItineraryWorkflowExtensions.cs | Updates agent instructions to reflect new JSON field names; tweaks itinerary planner temperature. |
| src/AI/samples/Essentials.AI.Sample/Views/Landmarks/LandmarkListItemView.xaml | UI polish for list item shadow/background + input transparency adjustments. |
| src/AI/samples/Essentials.AI.Sample/Views/Landmarks/LandmarkFeaturedItemView.xaml | UI polish for featured card shadow/background + input transparency adjustments. |
| src/AI/samples/Essentials.AI.Sample/Views/Landmarks/LandmarkHorizontalListView.xaml | Adds padding to avoid shadow clipping and align edge-to-edge scroller behavior. |
| src/AI/samples/Essentials.AI.Sample/Views/ChatOverlayView.xaml(.cs) | Updates bubble styling and scroll behavior to better follow streaming updates. |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/DayView.xaml | Adds WinUI placeholder for map and uses Background instead of BackgroundColor. |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/ActivityListView.xaml | Migrates from BackgroundColor to Background for borders. |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/ItineraryView.xaml | Updates itinerary rationale card styling and theme colors. |
| src/AI/samples/Essentials.AI.Sample/Resources/Styles/Styles.xaml | Removes global BoxView background style that interfered with gradients. |
| src/AI/samples/Essentials.AI.Sample/Resources/Styles/Colors.xaml | Adds missing Gray700 color resource. |
| src/AI/samples/Essentials.AI.Sample/Models/Landmark.cs | Removes per-landmark embedding storage. |
| src/AI/samples/Essentials.AI.Sample/Models/PointOfInterest.cs | Removes per-POI embedding storage. |
| src/AI/samples/Essentials.AI.Sample/Resources/Raw/landmarkData.json | Adds a new landmark entry (Bunaken Marine Park). |
| src/AI/samples/Essentials.AI.Sample/Resources/Raw/Images/Thumbnails/1023_thumb.jpg | Adds thumbnail for the new landmark. |
| src/AI/samples/Essentials.AI.Sample/Services/LanguagePreferenceService.cs | Deleted (language now selected in-detail and passed via navigation). |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/LandmarkTripView.xaml(.cs) | Deleted (replaced by updated TripPlanningPage). |
| src/AI/samples/Essentials.AI.Sample/Views/Itinerary/LandmarkDescriptionView.xaml(.cs) | Deleted (moved into new detail/flow). |
src/AI/tests/Essentials.AI.DeviceTests/Tests/ChatClientStreamingTestsBase.cs
Show resolved
Hide resolved
src/AI/tests/Essentials.AI.DeviceTests/Tests/ChatClientStreamingTestsBase.cs
Show resolved
Hide resolved
MauiBot
added
s/agent-changes-requested
- New LandmarkDetailPage with AI travel tips, similar destinations, tags - Semantic search bar on LandmarksPage with debounced filtering - ISemanticSearchService abstraction with EmbeddingSearchService and AppContentIndexerSearchService - Refactored DataService to use ISemanticSearchService instead of IEmbeddingGenerator - TaggingService switched to plaintext comma-separated parsing - TravelPlannerExecutor uses prompt-based JSON instead of structured output - TripPlanningPage auto-starts itinerary generation (no initial state) - Merged LandmarkTripView into TripPlanningPage, deleted unused views - Removed NonFunctionInvokingChatClient reference - Language picker moved to LandmarkDetailPage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix navigation crash: add missing Gray700 color to Colors.xaml - Fix gradient overlays: remove broken BoxView global style, use transparent-white gradients - Replace SearchBar with borderless Entry, remove native border/focus ring on MacCatalyst - Edge-to-edge horizontal scrollers with aligned padding - Scrolling gradient + solid background pattern on detail and trip planning pages - Safe area: use SafeAreaEdges=None on root Grids, Container on back buttons - Debounced search with Timer instead of per-keystroke async work - Use IDispatcher instead of MainThread.BeginInvokeOnMainThread - Only-once AI initialization on LandmarkDetailPage - Migrate BackgroundColor to Background across pages and views - Fix null dereference warnings in DataService Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… Bunaken landmark - PromptBasedSchemaClient middleware for prompt-based structured output - Fixed chat bubble alignment and scroll-to-end on Windows - Removed LanguagePreferenceService — language passed via navigation - Added Temperature for creative agents (0.8) and travel tips (0.9) - Fixed WeatherService locale issue (InvariantCulture) - Added weather display to LandmarkDetailPage - Map placeholder on Windows (OnPlatform) - TravelPlanResult uses short JsonPropertyName for SLM reliability - StreamingResponseHandler passthrough mode + 7 unit tests - Added Bunaken Marine Park (Manado, Indonesia) landmark - Fixed agent instruction examples Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tenatedText) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mattleibow
force-pushed
the
dev/ai-sample-improvements
branch
from
4d54ba8 to
b342d97
Compare
PromptBasedSchemaClient reads DisplayName for prompt-based JSON schema generation, which works across all platforms including Phi Silica. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🧪 PR Test EvaluationOverall Verdict: ✅ Tests are adequate The 7 new passthrough unit tests directly cover all modified code paths in
📊 Expand Full EvaluationPR Test Evaluation ReportPR: #34576 — Add passthrough mode to Overall Verdict✅ Tests are adequate The unit tests directly exercise all three modified code paths in 1. Fix Coverage — ✅The fix changes three code paths in
All three are exercised by the passthrough unit tests:
These tests would fail if the fix were reverted (e.g., a NullReferenceException on 2. Edge Cases & Gaps — ✅Covered:
Minor gaps (low priority):
3. Test Type Appropriateness — ✅Unit tests ( Device tests ( 4. Convention Compliance — ✅
No convention issues found. 5. Flakiness Risk — ✅ Low (unit) /
|
DisplayName does not work on properties for JSON schema generation — it only applies at the class/type level. JsonPropertyName is needed for PromptBasedSchemaClient to emit the correct short property names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🧪 PR Test EvaluationOverall Verdict: The unit tests for
📊 Expand Full EvaluationPR Test Evaluation ReportPR: #34576 — AI Sample improvements + Overall VerdictThe unit tests are excellent for the core fix. The two new device tests have weak assertions and flakiness risk that should be addressed before merge. 1. Fix Coverage — ✅The
The 39 sample app files have no tests, but this is appropriate — sample code does not typically require automated test coverage. 2. Edge Cases & Gaps —
|
| Test | Risk | Reason |
|---|---|---|
DeliversMultipleIncrementalUpdates |
Medium | Asserts Count > 2 on a real AI response. AI models may return large chunks reducing update count on slow hardware or with short context windows. |
ConcatenatedTextMatchesNonStreaming |
Medium | Makes two separate AI calls and asserts both are non-empty. Non-deterministic AI responses across calls make stronger equality assertions risky, but the current assertions don't actually test the stated invariant. |
6. Duplicate Coverage — ✅ No duplicates
No existing tests for passthrough mode existed before this PR. The new Passthrough.cs cleanly parallels the existing Content.cs test class (which tests the chunker-based path).
7. Platform Scope — ✅
- Unit tests: Run on all platforms — correct, since
StreamingResponseHandleris pure C#. - Device tests in
ChatClientStreamingTestsBase(T): Run on platforms with a concreteIChatClientimplementation (iOS/macOS for Apple Intelligence, Windows for Phi Silica). Cross-platform coverage matches the intent of the passthrough fix (Windows Phi Silica uses passthrough mode).
8. Assertion Quality — ⚠️
Unit tests: Strong ✅
Assert.Equal(2, updates.Count);
Assert.Equal("Hello", updates[0].Contents.OfType(TextContent)().Single().Text);Specific, will catch regressions.
Device tests: Weak
// ConcatenatedTextMatchesNonStreaming — the test name implies equality, but:
Assert.False(string.IsNullOrEmpty(streamingText.ToString()), "Streaming response should not be empty");
Assert.False(string.IsNullOrEmpty(nonStreamingText), "Non-streaming response should not be empty");These assertions only check non-empty strings — they don't verify the streaming and non-streaming results agree on content. The stated invariant ("Both should contain the answer (Paris)") is never actually asserted.
9. Fix-Test Alignment — ✅
The new Passthrough.cs unit tests target exactly StreamingResponseHandler with the new parameterless constructor. The device tests test the higher-level streaming API (GetStreamingResponseAsync) which ultimately flows through the fixed code on each platform. Alignment is good.
Recommendations
-
Fix
ConcatenatedTextMatchesNonStreamingassertions — either assert that both responses contain the expected answer (e.g.,Assert.Contains("Paris", streamingText.ToString(), StringComparison.OrdinalIgnoreCase)), or rename the test to accurately reflect what it tests. Currently the test name is misleading. -
Consider adding a tolerance to
DeliversMultipleIncrementalUpdates— theCount > 2threshold may be flaky on slower devices or with AI models that return large chunks. Consider lowering toCount > 1or adding a note explaining the threshold. -
Optional: Add a brief
ProcessToolResultsmoke test in passthrough mode — the method is unchanged but a test would confirm no regression during future refactors.
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
dc.services.visualstudio.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "dc.services.visualstudio.com"See Network Configuration for more information.
Note
🔒 Integrity filtering filtered 1 item
Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.
- pr:[AI] Sample: Detail page, semantic search, streaming, and UI polish #34576 (
pull_request_read: Resource 'pr:[AI] Sample: Detail page, semantic search, streaming, and UI polish #34576' has lower integrity than agent requires. Agent would need to drop integrity tags [unapproved:all approved:all] to trust this resource.)
🧪 Test evaluation by Evaluate PR Tests
🧪 PR Test EvaluationOverall Verdict: The
📊 Expand Full EvaluationPR Test Evaluation ReportPR: #34576 — AI Sample improvements + StreamingResponseHandler passthrough mode Overall VerdictThe core platform fix ( 1. Fix Coverage —
|
…nd test assertions - Replace IsNotNullConverter with new IsPositiveConverter for SimilarDestinations.Count bindings (int is never null, so IsNotNull always returned true even for 0 items) - Loosen streaming test assertion from >2 chunks to >=1 (streaming contract does not guarantee minimum chunk count) - Strengthen concatenated-text test to assert both responses contain 'Paris' instead of only checking non-empty Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mattleibow
force-pushed
the
dev/ai-sample-improvements
branch
from
3ad33f1 to
e61cdee
Compare
🧪 PR Test EvaluationOverall Verdict: ✅ Tests are adequate The
📊 Expand Full EvaluationPR Test Evaluation ReportPR: #34576 — AI sample improvements + Overall Verdict✅ Tests are adequate The core fix — adding a parameterless constructor to 1. Fix Coverage — ✅The
All 7 tests would fail to compile if the parameterless constructor were removed, proving strong coupling to the fix. The The 40 sample app files have no automated test coverage, but sample/demo code is not expected to have unit tests. 2. Edge Cases & Gaps — ✅Covered:
Potentially missing (minor):
3. Test Type Appropriateness — ✅Unit Tests ( Device Tests ( No suggestions to lighten — test types match the code being tested. 4. Convention Compliance — ✅Script detected 0 convention issues.
5. Flakiness Risk —
|
| Fix File | Tests It |
|---|---|
StreamingResponseHandler.cs (passthrough constructor + null guards) |
Passthrough.cs (7 unit tests) ✅ |
| Sample app files (40 files) | None — expected for demo/sample code ✅ |
| Platform ChatClient uses of passthrough mode | ChatClientStreamingTestsBase.cs (end-to-end) ✅ |
Recommendations
-
Low priority: In
ChatClientStreamingTestsBase.GetStreamingResponseAsync_UpdatesHaveContents, consider asserting that at least one content item is aTextContentrather than just checkingContents.Count > 0. This would better verify the streaming contract. -
Consider: Add a test for passthrough + cancellation token (
ReadAllAsynccancellation). The existingCancellation.cscovers only the chunker path, and while the channel behavior should be identical, explicit coverage would be more thorough. -
No action needed: The 40 sample app file changes without tests are appropriate — sample/demo code is expected to be untested.
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
dc.services.visualstudio.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "dc.services.visualstudio.com"See Network Configuration for more information.
Note
🔒 Integrity filtering filtered 1 item
Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.
- pr:[AI] Sample: Detail page, semantic search, streaming, and UI polish #34576 (
pull_request_read: Resource 'pr:[AI] Sample: Detail page, semantic search, streaming, and UI polish #34576' has lower integrity than agent requires. Agent would need to drop integrity tags [unapproved:all approved:all] to trust this resource.)
🧪 Test evaluation by Evaluate PR Tests
| var prompt = $""" | ||
| {searchContext}Write a brief, engaging 2-3 sentence travel tip for someone visiting {Landmark.Name} ({Landmark.Continent}). | ||
| Mention the best time to visit and one must-do activity. Be warm and inviting. | ||
| """; | ||
|
|
||
| var messages = new List<ChatMessage> | ||
| { | ||
| new(ChatRole.System, "You are a friendly travel guide who gives concise, helpful tips."), | ||
| new(ChatRole.User, prompt) | ||
| }; | ||
|
|
||
| var response = await chatClient.GetResponseAsync(messages, new ChatOptions { Temperature = 0.75f }, ct); | ||
| if (!ct.IsCancellationRequested) | ||
| AiTravelTip = response.Text; |
There was a problem hiding this comment.
SelectedLanguage is exposed (and the UI lets the user pick it), but GenerateAiTravelTipAsync doesn't use SelectedLanguage in the prompt, so the travel tip will always be generated in the model's default language. Incorporate SelectedLanguage into the prompt/system message (e.g., “Respond in {SelectedLanguage}”) and consider regenerating the tip when the language changes.
There was a problem hiding this comment.
Already fixed in the earlier commit (3ad33f1) — \SelectedLanguage\ is now used in the prompt.
| partial void OnSearchQueryChanged(string? value) | ||
| { | ||
| languagePreference.SelectedLanguage = value; | ||
| OnPropertyChanged(nameof(IsSearching)); | ||
| _debounceTimer?.Dispose(); | ||
| _debounceTimer = new Timer(_ => dispatcher.Dispatch(() => _ = FilterLandmarksAsync(value)), null, 300, Timeout.Infinite); | ||
| } | ||
|
|
||
| public async Task InitializeAsync() | ||
| { | ||
| if (IsLoading || ContinentGroups.Count > 0) | ||
| if (IsLoading || _allGroups.Count > 0) | ||
| return; | ||
|
|
||
| SelectedLanguage = languagePreference.SelectedLanguage; | ||
| await LoadLandmarksAsync(); | ||
| await WaitForEmbeddingsAsync(); | ||
| } | ||
|
|
||
| async Task FilterLandmarksAsync(string? query) | ||
| { | ||
| _searchCts?.Cancel(); | ||
| _searchCts = new CancellationTokenSource(); | ||
| var ct = _searchCts.Token; | ||
|
|
There was a problem hiding this comment.
OnSearchQueryChanged creates a new Timer on every keystroke. If the user navigates away while a timer is pending, the Timer can keep the view model rooted (and the callback can still run) because the timer/CTS are never disposed when the page is left. Consider adding a Dispose/Cancel method that disposes _debounceTimer and _searchCts, and call it from the page's NavigatingFrom/Unloaded handler.
There was a problem hiding this comment.
Fixed in aa6fa0f — added \CancelPendingSearch()\ to \LandmarksViewModel\ that disposes the timer and cancels the CTS. Called from \LandmarksPage.OnDisappearing().
| return responseText | ||
| .Split([',', '\n', '\r'], StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries) | ||
| .SelectMany(tag => tag.Split(' ', StringSplitOptions.RemoveEmptyEntries)) | ||
| .Select(tag => tag.TrimStart('#', '-', '•').Trim()) | ||
| .Where(tag => tag.Length > 1 && !tag.Contains(' ', StringComparison.Ordinal)) | ||
| .Distinct() | ||
| .Take(5) | ||
| .ToList(); |
There was a problem hiding this comment.
GenerateTagsAsync splits each tag on spaces (SelectMany(tag => tag.Split(' ', ...))). If the model returns a single hashtag phrase with spaces (or a leading bullet), this will produce multiple unrelated words that then pass the filters and show up as separate tags. Prefer treating each comma/newline-separated item as one tag; if it contains spaces after trimming, drop/normalize the whole tag instead of splitting it into words.
There was a problem hiding this comment.
Fixed in aa6fa0f — removed the \SelectMany\ space-split. Each comma/newline-separated item is now treated as one tag; entries containing whitespace are dropped instead of being fragmented into words.
| Padding="0" | ||
| StrokeShape="RoundRectangle 16" | ||
| StrokeThickness="0" | ||
| BackgroundColor="Transparent" | ||
| Shadow="{Shadow Brush=Black, Offset='0,2', Radius=8, Opacity=0.3}"> | ||
| <Border.GestureRecognizers> | ||
| <TapGestureRecognizer Tapped="OnLandmarkTapped" /> | ||
| </Border.GestureRecognizers> | ||
|
|
||
| <Grid> | ||
| <Grid InputTransparent="True"> | ||
| <Image |
There was a problem hiding this comment.
This introduces Border.BackgroundColor again, but the PR description calls out migrating from BackgroundColor to Background across the UI. Border supports Background (Brush), so consider using Background="Transparent" here for consistency with the rest of the PR and to avoid mixing the two patterns.
There was a problem hiding this comment.
Fixed in aa6fa0f — changed to \Background=\Transparent\ for consistency with the rest of the migration.
| Padding="0" | ||
| StrokeShape="RoundRectangle 16" | ||
| StrokeThickness="0" | ||
| BackgroundColor="Transparent" | ||
| Shadow="{Shadow Brush=Black, Offset='0,2', Radius=8, Opacity=0.3}"> | ||
| <Border.GestureRecognizers> | ||
| <TapGestureRecognizer Tapped="OnFeaturedLandmarkTapped" /> | ||
| </Border.GestureRecognizers> | ||
|
|
||
| <Grid HeightRequest="250"> | ||
| <Grid HeightRequest="250" InputTransparent="True"> | ||
| <Image |
There was a problem hiding this comment.
Same as the list item view: this reintroduces BackgroundColor on Border, while most of the PR migrates to the Background property. Consider using Background="Transparent" for consistency (and to keep the stated migration complete).
There was a problem hiding this comment.
Fixed in the same commit.
| /// <summary> | ||
| /// Result from the Travel Planner Agent - raw extraction of user intent. | ||
| /// These values should be extracted exactly as the user stated them, with no interpretation or expansion. | ||
| /// Short JSON names (dest/days/lang) prevent misspelling by small language models. | ||
| /// </summary> | ||
| public record TravelPlanResult( | ||
| [property: DisplayName("destinationName")] | ||
| [property: Description("The exact place/location name as written in the user's request. Extract the raw text only - do NOT interpret, expand, or look up actual landmarks. Example: 'Maui' not 'Maui, Hawaii' or 'Haleakala National Park'.")] | ||
| [property: JsonPropertyName("place")] | ||
| [property: Description("The destination name mentioned by the user.")] | ||
| string DestinationName, | ||
| [property: DisplayName("dayCount")] | ||
| [property: Description("The exact number of days mentioned by the user. Use 3 as default only if no number is specified.")] | ||
| [property: JsonPropertyName("days")] | ||
| [property: Description("Number of days for the trip. Default is 3.")] | ||
| int DayCount, | ||
| [property: DisplayName("language")] | ||
| [property: Description("The exact output language mentioned by the user. Use 'English' as default only if no language is specified.")] | ||
| [property: JsonPropertyName("language")] | ||
| [property: Description("Output language for the itinerary. Default is English.")] |
There was a problem hiding this comment.
The comment says “Short JSON names (dest/days/lang) prevent misspelling…”, but TravelPlanResult uses JsonPropertyName("place"), "days", and "language". Consider updating the remark to match the actual short names (place/days/language) to avoid confusion when editing prompts/schema.
There was a problem hiding this comment.
Fixed in aa6fa0f — updated comment to say \place/days/language\ to match the actual \JsonPropertyName\ values.
…nd test assertions - Replace IsNotNullConverter with new IsPositiveConverter for SimilarDestinations.Count bindings (int is never null, so IsNotNull always returned true even for 0 items) - Incorporate SelectedLanguage into AI travel tip prompt so the language picker on the detail page actually affects output - Loosen streaming test assertion from >2 chunks to >=1 (streaming contract does not guarantee minimum chunk count) - Strengthen concatenated-text test to assert both responses contain 'Paris' instead of only checking non-empty Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move the ISemanticSearchService registration from the iOS/MacCatalyst #if block into the common services section so it resolves on all platforms (Apple NL embeddings or OpenAI). This lets DataService take a required dependency instead of a nullable optional, removing the null-forgiving operators and the null guard in IndexContentAsync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🧪 PR Test EvaluationOverall Verdict: ✅ Tests are adequate The new
📊 Expand Full EvaluationPR Test Evaluation ReportPR: #34576 — AI Sample improvements + StreamingResponseHandler passthrough mode Overall Verdict✅ Tests are adequate The core library change — adding a parameterless constructor and null-guard passthrough mode to 1. Fix Coverage — ✅The only library change is
The 40 sample app files are UI/demo code; they don't need automated test coverage. 2. Edge Cases & Gaps —
|
| Changed Code Path | Covered By |
|---|---|
| Parameterless constructor | All passthrough tests |
ProcessContent: _chunker is not null ? ... : text |
ProcessContent_WithoutChunker_PassesDeltasDirectly, DoesNotAccumulateOrDelta |
ProcessContent: null guard |
ProcessContent_WithoutChunker_NullIgnored |
ProcessContent: empty string guard |
ProcessContent_WithoutChunker_EmptyStringIgnored |
ProcessToolCall: skip flush when no chunker |
ToolCall_WithoutChunker_NoFlushNeeded |
Complete: skip flush when no chunker |
Complete_WithoutChunker_NoExtraFlush |
CompleteWithError |
Error_WithoutChunker_PropagatesException |
Recommendations
-
Add
ProcessToolResultpassthrough test — the method is simple, but the suite tests every other passthrough path; a quick[Fact]forProcessToolResultwithout chunker would complete the coverage picture. -
Consider weakening or annotating the AI-response assertion in
GetStreamingResponseAsync_ConcatenatedTextMatchesNonStreaming— assertingContains("paris", ...)on a live AI model could be flaky. If flakiness occurs, consider using a prompt with a more constrained expected output, or add a[Trait("Flaky", "true")]annotation. -
Rename or strengthen
GetStreamingResponseAsync_DeliversMultipleIncrementalUpdates— the test asserts onlyCount >= 1but the name implies multiple incremental updates. Either assertCount > 1or rename to reflect the weaker assertion.
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
dc.services.visualstudio.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "dc.services.visualstudio.com"See Network Configuration for more information.
Note
🔒 Integrity filtering filtered 1 item
Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.
- pr:[AI] Sample: Detail page, semantic search, streaming, and UI polish #34576 (
pull_request_read: Resource 'pr:[AI] Sample: Detail page, semantic search, streaming, and UI polish #34576' has lower integrity than agent requires. Agent would need to drop integrity tags [unapproved:all approved:all] to trust this resource.)
🧪 Test evaluation by Evaluate PR Tests
- Add CancelPendingSearch to LandmarksViewModel, called from OnDisappearing to dispose the debounce timer and cancel any in-flight search when navigating away - Fix TaggingService tag parsing: drop the SelectMany space-split that fragmented multi-word hashtags; instead treat each comma/newline-separated item as a single tag and reject entries with whitespace - Use Background instead of BackgroundColor on LandmarkListItemView and LandmarkFeaturedItemView for consistency with the rest of the BackgroundColor-to-Background migration - Fix WorkflowModels comment to match actual JSON property names (place/days/language not dest/days/lang) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e tip, and test assertions" This reverts commit 1bd8424.
🧪 PR Test EvaluationOverall Verdict: ✅ Tests are adequate The unit tests for
📊 Expand Full EvaluationPR Test Evaluation ReportPR: #34576 — AI Sample Improvements Overall Verdict✅ Tests are adequate The one non-sample library change — adding passthrough mode to 1. Fix Coverage — ✅The key library fix makes
The The 40 sample app changes (ViewModels, Pages, Services, XAML) are UI/sample code that is not typically covered by automated tests, which is appropriate. 2. Edge Cases & Gaps — ✅Covered:
Minor gaps (non-critical):
3. Test Type Appropriateness — ✅
The device test is an abstract base class inherited by 4. Convention Compliance — ✅
5. Flakiness Risk —
|
| Code change | Test coverage |
|---|---|
New StreamingResponseHandler() constructor |
All 7 passthrough tests |
ProcessContent: _chunker is not null ? ... : text |
PassesDeltasDirectly, DoesNotAccumulateOrDelta, NullIgnored, EmptyStringIgnored |
ProcessToolCall: null-guard for _chunker |
ToolCall_WithoutChunker_NoFlushNeeded |
Complete: null-guard for _chunker |
Complete_WithoutChunker_NoExtraFlush |
CompleteWithError: (unchanged) |
Error_WithoutChunker_PropagatesException |
Recommendations
- Consider adding
[Category]to concrete streaming device test classes —AppleIntelligenceChatClientStreamingTestsandOpenAIChatClientStreamingTestsdon't appear to have[Category]attributes (unlike sibling test classes), which could affect CI filtering. - Long-term: A device test for Windows Phi Silica streaming (passthrough path end-to-end) would validate the full integration, but this is non-blocking given the thorough unit test coverage.
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
dc.services.visualstudio.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "dc.services.visualstudio.com"See Network Configuration for more information.
Note
🔒 Integrity filtering filtered 1 item
Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.
- pr:[AI] Sample: Detail page, semantic search, streaming, and UI polish #34576 (
pull_request_read: Resource 'pr:[AI] Sample: Detail page, semantic search, streaming, and UI polish #34576' has lower integrity than agent requires. Agent would need to drop integrity tags [unapproved:all approved:all] to trust this resource.)
🧪 Test evaluation by Evaluate PR Tests
|
/azp run maui-pr-devicetests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
iOS does not fall back to Apple Color Emoji when a custom FontFamily (OpenSansRegular) is set via global styles, causing all emoji to render as [?] replacement characters. Replace all emoji usage with FluentSystemIcons-Regular font glyphs: - Chat FAB, header, delete, close, send buttons - Back button FABs on detail and trip planning pages - Weather icons (split into separate icon + text labels) - Sparkle/AI indicators - Globe and map icons on language picker and plan trip button - Activity bullet points replaced with location pin icons Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines successfully started running 1 pipeline(s). |
The MauiFont glob (Resources/Fonts/*) was consuming FluentUI.cs as a font asset instead of a Compile item, causing CS0103 errors on Windows and other CI platforms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🧪 PR Test EvaluationOverall Verdict: The new passthrough mode in
📊 Expand Full EvaluationPR Test Evaluation ReportPR: #34576 — AI Sample improvements (streaming handler passthrough mode + sample app polish) Overall VerdictThe new passthrough mode added to 1. Fix Coverage — ✅The one substantive production code change — adding passthrough mode to
The device test base class ( The 44 sample app changes have no automated tests — this is normal for demo/sample code. 2. Edge Cases & Gaps —
|
| Test | Type | Verdict |
|---|---|---|
Passthrough.cs |
Unit | ✅ Correct — pure in-memory logic, no platform dependency |
ChatClientStreamingTestsBase.cs |
Device | ✅ Correct — tests live AI model responses, requires real platform runtime |
The unit tests are well-suited for StreamingResponseHandler (a POCO with no platform dependencies). Device tests are necessary to validate that the streaming pipeline works end-to-end with actual AI backends.
4. Convention Compliance — ✅
- Unit tests use
[Fact]✅ - Nested class structure (
PassthroughTestsinsideStreamingResponseHandlerTests) follows existing patterns in the codebase ✅ - No convention issues detected by the automated script ✅
- Device test base class follows the established
ChatClient*TestsBase(T)pattern already used for function calling, cancellation, etc. ✅
5. Flakiness Risk — ⚠️ Medium
Unit tests (Passthrough.cs): ✅ Very low — pure in-memory operations with deterministic behavior.
Device tests (ChatClientStreamingTestsBase):
- Tests depend on live AI model responses (Apple Intelligence, OpenAI). If the AI service is unavailable or returns unexpected content, tests fail.
ConcatenatedTextMatchesNonStreamingassertsContains("paris", ...)— reasonable but could fail if the model responds with "Paris, France" vs "Paris" in different configurations, or if the model's output format changes.DeliversMultipleIncrementalUpdatesassertsCount >= 1— very lenient, but the comment acknowledges that the streaming contract doesn't guarantee a minimum chunk count. This is intentionally loose to avoid brittleness, but it means the test might not catch regressions in streaming granularity.
6. Duplicate Coverage — ✅ No duplicates
No similar tests found. The Passthrough.cs tests specifically address the new passthrough mode, which had no prior coverage. The new device test methods extend the existing streaming base class without duplicating existing tests.
7. Platform Scope — ✅
StreamingResponseHandleris cross-platform (no platform-specific code).- Unit tests run on all platforms.
- Device tests run on Apple (iOS/macCatalyst) via
AppleIntelligenceChatClientStreamingTestsand can run with OpenAI on any platform viaOpenAIChatClientStreamingTests.
8. Assertion Quality — ✅
Unit tests have specific, targeted assertions:
Assert.Equal(2, updates.Count)— exact count verification ✅Assert.Equal("Hello", updates[0].Contents.OfType(TextContent)().Single().Text)— exact text match ✅Assert.IsType(FunctionCallContent)(...)— type-specific assertion ✅
Device tests use appropriately loose assertions for live AI responses:
Assert.Contains("paris", ..., OrdinalIgnoreCase)— reasonable for non-mocked responses ✅Assert.True(updates.Count >= 1, ...)— intentionally lenient, with explanatory message ✅
No magic numbers or trivial Assert.True(true) assertions found.
9. Fix-Test Alignment — ✅
The tests directly target the changed production code:
Passthrough.cs→ testsStreamingResponseHandlerpassthrough constructor and all methodsChatClientStreamingTestsBase.cs→ tests the streaming interface thatStreamingResponseHandlerbacks
The sample app files (44 of 45 "fix" files) have no corresponding automated tests, which is appropriate for sample/demo code.
Recommendations
-
Add a passthrough-mode
ProcessToolResulttest — Even though the code path is transitively covered by chunker-mode tests, an explicit passthrough test would clarify intent and protect against future regressions if branching is added. Example: adaptToolResultEdgeCasesto include a version withnew StreamingResponseHandler()(no chunker). -
Consider adding a cancellation test for passthrough mode — Check whether
Cancellation.cscovers the no-chunker constructor path. If not, add one test to ensureReadAllAsync(cancellationToken)respects cancellation in passthrough mode. -
Document the live-AI flakiness expectation — The device tests (
DeliversMultipleIncrementalUpdates,ConcatenatedTextMatchesNonStreaming) rely on live AI responses. Consider adding a[Skip]or[Category]annotation that marks them as integration/live tests, so CI can conditionally skip them without AI credentials.
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
dc.services.visualstudio.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "dc.services.visualstudio.com"See Network Configuration for more information.
Note
🔒 Integrity filtering filtered 1 item
Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.
- pr:[AI] Sample: Detail page, semantic search, streaming, and UI polish #34576 (
pull_request_read: Resource 'pr:[AI] Sample: Detail page, semantic search, streaming, and UI polish #34576' has lower integrity than agent requires. Agent would need to drop integrity tags [unapproved:all approved:all] to trust this resource.)
🧪 Test evaluation by Evaluate PR Tests
|
Azure Pipelines successfully started running 1 pipeline(s). |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines successfully started running 1 pipeline(s). |
The Essentials.AI.UnitTests project links Models/** from the sample via Compile Include glob. Weather.cs referencing FluentUI broke that project since it doesn't have FluentUI.cs. Move icon mapping to WeatherService and keep the model as pure data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines successfully started running 1 pipeline(s). |
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
Major improvements to the Essentials AI sample app: new Landmark Detail page with AI features, semantic search, streaming response handler improvements, and comprehensive UI polish across all pages for a modern, edge-to-edge experience on iOS and MacCatalyst.
New Features
Timer-based debounce (300ms) and tracks recent searches for contextual AI descriptionsEmbeddingSearchService(Apple NL embeddings + cosine similarity + hybrid keyword boost + sentence chunking)DeliversMultipleIncrementalUpdates,ConcatenatedText)UI Polish
SafeAreaEdges="None"on root Grid with back buttons wrapped inSafeAreaEdges="Container"SearchBarwith borderlessEntryin roundedBorderwith native border/focus ring removed on iOS/MacCatalystBackgroundColorwithBackgroundacross all pagesCode Quality
MainThread.BeginInvokeOnMainThreadLandmarkDetailViewModelNavigation Flow
LandmarksPage(browse + search) →LandmarkDetailPage(details + AI tips) →TripPlanningPage(itinerary generation)Deleted Files
LandmarkDescriptionView,LandmarkTripView— Replaced by new pagesLanguagePreferenceService— Refactored into inline language arrayNew Test Coverage
StreamingResponseHandlerTests/Passthrough.cs— Unit tests for passthrough streaming modeChatClientStreamingTestsBase— Updated device tests for streaming scenarios