Architecture Cleanup#47
Merged
MikeAlhayek merged 24 commits intomainfrom Apr 24, 2026
Merged
Conversation
1. YesSql PageAsync: Push sort/pagination to database instead of in-memory LINQ 2. NamedAICompletionClient: Materialize chatMessages to avoid double enumeration 3. AIFunctionArgumentsExtensions: Replace bare catch with catch(Exception) 4. LuceneTextTokenizer: Convert Regex.Compiled to [GeneratedRegex] source generator 5. TabularBatchResultCache: Replace sync cache calls with async (GetAsync/SetAsync/RemoveAsync) 6. StreamExtensions: Reset stream position before CopyTo if seekable 7. DefaultAIDataSourceIndexingService: Make BuildChunkIds max configurable via parameter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Cache OpenAI SDK client instances in OpenAIClientProvider using ConcurrentDictionary - Cache AzureOpenAI client instances in AzureOpenAIClientFactory using ConcurrentDictionary - Cache OllamaApiClient instances in OllamaAIClientProvider using ConcurrentDictionary - Add scope-level template caching in DefaultTemplateService.GetAsync - Extract ResolveClientAsync helper in DefaultAIClientFactory to eliminate 5x duplicated foreach loop - Consolidate 3 identical completion client classes (OpenAI, Ollama, AzureAIInference) into generic ProviderAICompletionClient<TProvider> with IAIProviderMarker interface Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add It.IsAny<CancellationToken>() to Moq Setup, Callback, and Verify calls that reference interface methods which now accept CancellationToken. Fixed files: - MvcAIDocumentIndexingServiceTests: FindByNameAsync setups and verifies - AIMemoryIndexingServiceTests: FindByNameAsync and FindByIdAsync setups - AIMemorySearchServiceTests: FindByNameAsync and FindByIdAsync setups - YesSqlAIChatSessionManagerTests: CreateAsync verify - DataSourceChatInteractionSettingsHandlerTests: FindByIdAsync setup - DefaultAIProfileTemplateManagerTests: GetAllAsync, FindByIdAsync, and GetTemplatesAsync setups Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix FTP/SFTP path traversal vulnerability by adding SanitizePath to McpResourceTypeHandlerBase that rejects '..' sequences and null bytes - Hash API keys before using as cache keys in OpenAIClientProvider to prevent plaintext secrets in memory dumps - Fix YesSql DeleteAllAsync/DeleteAsync to also delete associated prompt records, preventing orphaned data (matching EntityCore behavior) - Fix YesSql FindAsync to delegate to FindByIdAsync instead of duplicating - Fix incorrect interface signatures in documentation: - IODataValidator: TryValidate -> IsValidFilter - IAICompletionService: ChatOptions -> AICompletionContext, correct param types - IAICompletionClient: add ClientName property, messages parameter, correct types - AITool.InvokeCoreAsync: Task<object> -> ValueTask<object> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add Microsoft.Extensions.Http.Resilience with standard resilience handlers to A2A, Copilot, and MCP HTTP clients for automatic retry with backoff, circuit breaker, and timeout policies on external API calls - Use named HttpClients (CrestApps.A2A, CrestApps.Copilot, CrestApps.Mcp) instead of anonymous clients for proper resilience pipeline binding - Add HttpClientName constants to A2AConstants, McpConstants, and CopilotOrchestrator for consistent named client usage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…fixes Short-term improvements: - Add IValidateOptions for ClaudeOptions and CopilotOptions to fail fast on missing API keys or invalid authentication configuration at startup - Add ClearCache() methods to OpenAIClientProvider, OllamaAIClientProvider, and AzureOpenAIClientFactory for credential rotation support - Add InternalsVisibleTo for CrestApps.Core.AI and CrestApps.Core.AI.Chat to enable test access without reflection - Add xunit.analyzers to test project for catching common test pitfalls Miscellaneous fixes: - Fix grammar: 'does not exists' -> 'does not exist' in DictionaryExtensions - Use const instead of static readonly for DataSourceConstants.IndexingTaskType - Fix stale OrchardCore.Entities.Entity.Properties references in XML docs - Preserve DateTimeKind in DateTimeExtensions methods - Make Str and NumberHelpers classes static - Remove duplicate XML summary in ISearchIndexProfileStore Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…catalog interfaces - Remove unused utility methods from Str.cs and StringExtensions.cs - Delete unused files: ExpressionHelper.cs, TypeHelpers.cs, NumberHelpers.cs, QueryHelpers.cs, StreamExtensions.cs - Add glossary page with key CrestApps.Core terminology - Add Hello World example to getting-started guide - Add granular ICatalogEventHandler interfaces (Creating, Created, Updating, Updated, Deleting, Deleted, Validating, Validated) - Add ai-profiles and glossary to docs sidebar - Fix broken Redis JSON in signalr.md - Fix stale Blazor reference in architecture.md - Set onBrokenLinks to throw in docusaurus.config.js Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… base class Refactor 7 standalone YesSql stores to extend DocumentCatalog<T, TIndex> instead of reimplementing identical CRUD methods. This eliminates ~500 lines of copy-pasted code while preserving each store's domain-specific queries. Stores refactored: - YesSqlAIDataSourceStore (pure CRUD, now ~8 lines) - YesSqlAIDocumentStore (1 unique method retained) - YesSqlAIDocumentChunkStore (3 unique methods retained) - YesSqlAIMemoryStore (3 unique methods retained) - YesSqlAIChatSessionPromptStore (3 unique methods retained) - YesSqlChatInteractionPromptStore (2 unique methods retained) - YesSqlSearchIndexProfileStore (2 unique methods retained) Benefits: - GetAllAsync now has 10K safety cap (was unbounded) - PageAsync now supports QueryContext filtering - CreateAsync/UpdateAsync gain SavingAsync hook support - DeleteAsync gains DeletingAsync hook support Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract duplicated authentication header building logic (~160 lines per project) into a shared IConnectionAuthHeaderBuilder service in CrestApps.Core.AI. Both MCP SSE and A2A now delegate to the shared DefaultConnectionAuthHeaderBuilder. New shared types: - ClientAuthenticationType enum (replaces duplicate enums) - IConnectionAuthMetadata interface (implemented by both metadata classes) - IConnectionAuthHeaderBuilder + DefaultConnectionAuthHeaderBuilder Changes: - SseMcpConnectionMetadata implements IConnectionAuthMetadata - A2AConnectionMetadata implements IConnectionAuthMetadata - SseClientTransportProvider reduced from 109 to 30 lines - DefaultA2AConnectionAuthService reduced from 165 to 32 lines - Protocol-specific enums retained for backward compatibility Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 13 tests covering all authentication types for the new shared connection auth header builder: Anonymous, ApiKey (default/custom header/prefix/empty), Basic (normal/empty password/empty username), OAuth2 Client Credentials (success/missing fields), and Custom Headers (normal/null dictionary). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- replace A2AClientAuthenticationType and McpClientAuthenticationType with ClientAuthenticationType across protocol metadata, MVC/Blazor hosts, tests, and docs - delete the protocol-specific auth enums after consolidating all callers - add AzureOpenAIClientMarker as safe groundwork for future generic pipeline alignment without changing AzureOpenAICompletionClient behavior - update changelog and copilot instructions with the new shared-abstraction guidance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
No description provided.