Adds Provider interfaces #35
Conversation
|
I did add some methods to |
Add core Provider Registry functionality that enables provider discovery and model selection based on requirements. This is a critical MVP component that unlocks the main AiClient functionality. **Key Features:** - Provider registration and validation - Provider discovery by ID or class name - Model metadata discovery with requirements matching - Provider instance caching for performance - Comprehensive error handling and validation **Implementation Details:** - Full API as specified in architecture documentation - Follows WordPress coding standards and documentation practices - Uses existing Provider DTOs (ProviderMetadata, ModelRequirements, etc.) - Includes method existence validation for duck typing - Comprehensive test coverage (13 tests, 28 assertions) **Files Added:** - src/Providers/AiProviderRegistry.php - Main registry implementation - tests/unit/Providers/AiProviderRegistryTest.php - Full test suite - tests/unit/Providers/MockProvider.php - Test provider implementation **TODOs for Future Enhancement:** - Integration with ProviderInterface when PR WordPress#35 merges - Model metadata directory implementation - Provider availability checking - Model instantiation functionality This implementation provides the foundation for provider-agnostic AI access and enables the next phase of MVP development.
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
@JasonTheAdams This looks almost good to go, just a few small points of feedback.
I like the addition of ModelMetadata::meetsRequirements 👍
| /** | ||
| * Interface for models that support generative AI operations. | ||
| * | ||
| * Provides methods to retrieve and manage long-running generative AI | ||
| * operations such as text, image, or speech generation. | ||
| * | ||
| * @since n.e.x.t | ||
| */ | ||
| interface WithGenerativeAiOperationsInterface |
There was a problem hiding this comment.
I'm starting to question whether this approach makes sense: Looking up an existing operation is technically subject to the provider support, not model-specific, so maybe this shouldn't be an interface for models to implement?
Maybe we could instead have a ProviderOperationsHandlerInterface (something like that) with the relevant instance methods (like getOperation), and another ProviderWithOperationsInterface which the actual provider class would implement, with a single static method operationsHandler() which would return the provider-specific instance. That would align well with how the rest works, but keep it optional, as not every provider would need this.
LMKWYT. If this needs more thinking and more discussion, we could leave it out of this PR entirely, as we'll only implement long-running operations later anyway. The key features for the MVP and the big three providers don't need it.
There was a problem hiding this comment.
You know more than I do in this case, but are we confident that looking up operations will always be on a per-provider and not per-model basis? I'd hate to make assumptions that back us into a corner where a provider exists with varying model operational abilities.
I do agree, though, that we shouldn't spend too much time on this right now.
There was a problem hiding this comment.
All existing long-running operation implementations in APIs I've seen are model independent - i.e. tied to the entire provider, not specific models. It also makes sense conceptually because the model is only relevant once you trigger/start the operation. At that point you pass all the same information you'd pass to a regular request (model ID, model config etc.), but after that you just get an operation ID back that you can look up provider-wide - no need to remember which model was used at that point.
From that perspective, I'd suggest we go with what I mentioned in the previous comment:
ProviderOperationsHandlerInterfacewith (non-static) methodgetOperation(just like the current interface)ProviderWithOperationsHandlerInterfacewith static methodoperationsHandler(): ProviderOperationsHandlerInterface
Would you be okay replacing this current interface with that? This way we have a starting point in place for operations, which IMO makes sense for a comprehensive picture (similar to #35 (comment)), even if we'll only use it later.
There was a problem hiding this comment.
Gotcha! Appreciate the insight! I resolved this in 5b46445
| /** | ||
| * Streams text generation from a prompt. | ||
| * | ||
| * @since n.e.x.t | ||
| * | ||
| * @param list<Message> $prompt Array of messages containing the text generation prompt. | ||
| * @return Generator<GenerativeAiResult> Generator yielding partial results. | ||
| */ | ||
| public function streamGenerateTextResult(array $prompt): Generator; |
There was a problem hiding this comment.
Since we don't likely want to implement streaming for the MVP yet (due to additional complexity), we may want to leave this out for now.
That being said, having this already in place clarifies the intention and leaves the decision to implement it or not at the provider level, so I kind of like keeping it despite the above. Initially we could just have it throw a "Not implemented" exception, until we get to it.
WDYT?
There was a problem hiding this comment.
I'm good with leaving it and throwing an exception. It will be asked about, otherwise. 😆
Co-authored-by: Felix Arntz <[email protected]>
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
@JasonTheAdams Looks all good, pending only #35 (comment).
…ss#35 This commit transforms the Registry from placeholder implementation to fully functional provider orchestration system: • Add all Provider interface contracts from PR WordPress#35 • Update Registry to use real interface methods instead of TODOs • Enhance ModelMetadata with meetsRequirements() method for intelligent selection • Create complete mock provider ecosystem for comprehensive testing • Fix all PHPStan type safety and PHPCS style compliance issues • Achieve 100% test coverage with 548 passing tests Registry is now production-ready and will work immediately when PR WordPress#35 merges. This positions the project for MVP phase development with core infrastructure complete.
Add core Provider Registry functionality that enables provider discovery and model selection based on requirements. This is a critical MVP component that unlocks the main AiClient functionality. **Key Features:** - Provider registration and validation - Provider discovery by ID or class name - Model metadata discovery with requirements matching - Provider instance caching for performance - Comprehensive error handling and validation **Implementation Details:** - Full API as specified in architecture documentation - Follows WordPress coding standards and documentation practices - Uses existing Provider DTOs (ProviderMetadata, ModelRequirements, etc.) - Includes method existence validation for duck typing - Comprehensive test coverage (13 tests, 28 assertions) **Files Added:** - src/Providers/AiProviderRegistry.php - Main registry implementation - tests/unit/Providers/AiProviderRegistryTest.php - Full test suite - tests/unit/Providers/MockProvider.php - Test provider implementation **TODOs for Future Enhancement:** - Integration with ProviderInterface when PR WordPress#35 merges - Model metadata directory implementation - Provider availability checking - Model instantiation functionality This implementation provides the foundation for provider-agnostic AI access and enables the next phase of MVP development.
…ss#35 This commit transforms the Registry from placeholder implementation to fully functional provider orchestration system: • Add all Provider interface contracts from PR WordPress#35 • Update Registry to use real interface methods instead of TODOs • Enhance ModelMetadata with meetsRequirements() method for intelligent selection • Create complete mock provider ecosystem for comprehensive testing • Fix all PHPStan type safety and PHPCS style compliance issues • Achieve 100% test coverage with 548 passing tests Registry is now production-ready and will work immediately when PR WordPress#35 merges. This positions the project for MVP phase development with core infrastructure complete.
Add core Provider Registry functionality that enables provider discovery and model selection based on requirements. This is a critical MVP component that unlocks the main AiClient functionality. **Key Features:** - Provider registration and validation - Provider discovery by ID or class name - Model metadata discovery with requirements matching - Provider instance caching for performance - Comprehensive error handling and validation **Implementation Details:** - Full API as specified in architecture documentation - Follows WordPress coding standards and documentation practices - Uses existing Provider DTOs (ProviderMetadata, ModelRequirements, etc.) - Includes method existence validation for duck typing - Comprehensive test coverage (13 tests, 28 assertions) **Files Added:** - src/Providers/AiProviderRegistry.php - Main registry implementation - tests/unit/Providers/AiProviderRegistryTest.php - Full test suite - tests/unit/Providers/MockProvider.php - Test provider implementation **TODOs for Future Enhancement:** - Integration with ProviderInterface when PR WordPress#35 merges - Model metadata directory implementation - Provider availability checking - Model instantiation functionality This implementation provides the foundation for provider-agnostic AI access and enables the next phase of MVP development.
…ss#35 This commit transforms the Registry from placeholder implementation to fully functional provider orchestration system: • Add all Provider interface contracts from PR WordPress#35 • Update Registry to use real interface methods instead of TODOs • Enhance ModelMetadata with meetsRequirements() method for intelligent selection • Create complete mock provider ecosystem for comprehensive testing • Fix all PHPStan type safety and PHPCS style compliance issues • Achieve 100% test coverage with 548 passing tests Registry is now production-ready and will work immediately when PR WordPress#35 merges. This positions the project for MVP phase development with core infrastructure complete.
This adds the contracts (interfaces) for the Providers domain. It omits:
The latter two will be coming in a subsequent PR.