-
Notifications
You must be signed in to change notification settings - Fork 0
feat(conversation): introduce ConversationSession; refactor AICall, streaming, validation, and model selection #315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ts with observers and multi-turn flows and tool passes
…/tools - Remove legacy AICall types under Infrastructure/AICall/* - Update components, GH tools, and providers to new Core namespaces - Tweak ConversationSession and interfaces accordingly - Refresh docs/Tools/index.md and CHANGELOG entries
… EnableStreaming flag and update docs/tests
…chema via request policy
…d simplify AIToolCall
…ified replaced and invalid
…een generation and backoffice calls which validate differently
|
🏷️ This PR has been automatically assigned to milestone 0.6.0-alpha based on the version in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a major architectural refactor focused on conversation orchestration, streaming capabilities, and centralized validation. The changes implement ConversationSession as the primary orchestrator for multi-turn conversations with tool execution, while maintaining backward compatibility through existing AIRequestCall for single-turn operations.
Key changes include:
- ConversationSession architecture: New session-based orchestration with observer patterns for multi-turn conversations and tool execution
- Unified streaming framework: Provider-agnostic streaming adapters with centralized capability checks and consistent delta emission
- Centralized validation and policy pipeline: Request/response policies for schema validation, tool validation, and response normalization
Reviewed Changes
Copilot reviewed 164 out of 164 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Provider settings files | Added EnableStreaming setting across OpenAI, MistralAI, and DeepSeek providers |
| Provider model files | Updated model capabilities and added RetrieveApiModels() method implementations |
| Provider main files | Added streaming adapters and centralized JSON schema handling |
| JSON schema adapters | New provider-specific schema wrapping/unwrapping logic |
| Assistant settings | Added canvas button toggle and reorganized settings structure |
| Streaming infrastructure | New IStreamingAdapter interface and base classes for provider streaming |
| Validation framework | Comprehensive tool and schema validators with structured error reporting |
| Session management | ConversationSession implementation for multi-turn orchestration |
| Policy pipeline | Request/response policies for centralized validation and normalization |
| Metrics framework | Enhanced metrics tracking with correlation and domain-specific records |
Comments suppressed due to low confidence (1)
src/SmartHopper.Infrastructure/AICall/Sessions/ConversationSession.cs:1
- The timeout logic creates a potential memory leak as the original execTask continues running even after timeout. Consider implementing proper cancellation token propagation to AIToolManager.ExecuteTool() to actually cancel the underlying work.
/*
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| }; | ||
|
|
||
| // Rebuild body immutably with context as first item and filter-out any pre-existing context interactions | ||
| var rest = body.Interactions?.Where(i => i != null && i.Agent != AIAgent.Context) ?? Enumerable.Empty<IAIInteraction>(); |
Copilot
AI
Aug 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LINQ Where clause filters out existing context interactions, but this creates a new enumerable each time. Consider materializing this to a list if the collection will be enumerated multiple times in AddRange().
| var rest = body.Interactions?.Where(i => i != null && i.Agent != AIAgent.Context) ?? Enumerable.Empty<IAIInteraction>(); | |
| var rest = body.Interactions?.Where(i => i != null && i.Agent != AIAgent.Context).ToList() ?? new List<IAIInteraction>(); |
| if (interaction is AIInteractionToolCall || interaction is AIInteractionToolResult) | ||
| { | ||
| this.Request.Body = this.Request.Body.WithAppended(interaction); | ||
| } |
Copilot
AI
Aug 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During streaming, the request body is being rebuilt repeatedly with each interaction. This creates multiple immutable copies. Consider batching these updates or using a more efficient accumulation strategy.
| /// </summary> | ||
| public static class JsonSchemaAdapterRegistry | ||
| { | ||
| private static readonly ConcurrentDictionary<string, IJsonSchemaAdapter> _adapters = new ConcurrentDictionary<string, IJsonSchemaAdapter>(StringComparer.OrdinalIgnoreCase); |
Copilot
AI
Aug 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The static ConcurrentDictionary for adapter registration could lead to issues in testing scenarios where adapters need to be reset between tests. Consider providing a Reset() method for test scenarios or making the registry injectable.
Description
This PR delivers a major architectural refactor and feature set focused on conversation orchestration, centralized validation, streaming, and model management. Changes are summarized and grouped below based on commit messages and the [Unreleased] section in CHANGELOG.md.
IConversationSession,IConversationObserver,SessionOptions; multi‑turn and tool orchestration with observer callbacks; single source of truth for long‑running interactions.AIBodyBuilder;AIRequestKindto distinguish generation vs backoffice flows.AIRequestCall.Exec()clarified as single‑turn; multi‑turn flows handled via ConversationSession.AIProviderStreamingAdapter; OpenAI streaming adapter now derives from the shared base.ModelManager.ModelSupportsStreaming(...).ConversationSession.Stream()clearly gates unsupported models and emits actionable errors.AIMessageCodesurfaced throughAIRuntimeMessageand consumed by components (e.g., badges).IAIProvider.SelectModel(...)delegates to centralizedModelManager.SelectBestModelwhile honoring provider defaults/settings.IAIProviderModels.RetrieveModels()and registered centrally.EnableStreaming.list_generateresult output and image output propagation toImageViewer.Breaking Changes
AIRequestCall.Exec()is now explicitly single‑turn; multi‑turn/tool passes requireConversationSession.RetrieveAvailable,RetrieveCapabilities,RetrieveDefaultin favor ofRetrieveModels()and centralizedModelManager.TemplateProviderandAIToolCall.ReplaceReuseCount()(unified metrics handling).Testing Done
ImageViewer.Checklist