Feat/add ai.chat.allowed models feature limit #552
Conversation
…sage - AIController.ts now rejects requests if the requested model is not in the allowed models list - Added error handling to provide feedback when disallowed model is requested
…llowedModels_feature_limit
chore: Added description to AIChatFeaturesConfiguration allowedModels interface
TroyceGowdy
added
enhancement
src/aux-records/AIController.spec.ts
Outdated
| const model = 'modelA'; | ||
| const result = { choices: ['choice1', 'choice2'] }; | ||
|
|
||
| const response = (function () { |
There was a problem hiding this comment.
@TroyceGowdy These tests need to actually call into the AIController in order to properly test that the AIController is functioning correctly. Currently, the test just happens to use the same code as the AIController, but there is no guarantee that it will stay that way in the future,
…icate code. - Updated implementation to use AIController.ts - Removed redundant code that was duplicating AIController.ts functionality
src/aux-records/AIController.spec.ts
Outdated
| it('should return success when allowedModels includes the model', async () => { | ||
| chatInterface.chat.mockReturnValueOnce( | ||
| Promise.resolve({ | ||
| choices: [ | ||
| { | ||
| role: 'user', | ||
| content: 'test', | ||
| finishReason: 'stop', | ||
| }, | ||
| ], | ||
| totalTokens: 1, | ||
| }) | ||
| ); | ||
|
|
||
| const result = await controller.chat({ | ||
| model: 'test-model1', | ||
| messages: [ | ||
| { | ||
| role: 'user', | ||
| content: 'test', | ||
| }, | ||
| ], | ||
| temperature: 0.5, | ||
| userId, | ||
| userSubscriptionTier, | ||
| }); |
There was a problem hiding this comment.
@TroyceGowdy This test should be inside the "subscriptions" group and it should setup a subscription configuration that specifies the allowed models for the subscription tier that the user has. See lines 626-649 for an example of setting up the subscription configuration.
The way this test is currently written, it doesn't actually test the new subscription feature limit and only tests the server config.
| it('should return not_authorized when allowedModels does not include the model', async () => { | ||
| controller = new AIController({ | ||
| chat: { | ||
| interfaces: { | ||
| provider1: chatInterface, | ||
| }, | ||
| options: { | ||
| defaultModel: 'default-model', | ||
| defaultModelProvider: 'provider1', | ||
| allowedChatModels: [ | ||
| { | ||
| provider: 'provider1', | ||
| model: 'modelA', | ||
| }, | ||
| { | ||
| provider: 'provider1', | ||
| model: 'modelB', | ||
| }, | ||
| ], | ||
| allowedChatSubscriptionTiers: ['test-tier'], | ||
| }, | ||
| }, | ||
| generateSkybox: null, | ||
| images: null, | ||
| metrics: store, | ||
| config: store, | ||
| hume: null, | ||
| sloyd: null, | ||
| policies: null, | ||
| policyController: policies, | ||
| records: store, | ||
| }); |
There was a problem hiding this comment.
@TroyceGowdy Same with this test. You shouldn't have to create a new AIController, instead you need to setup a subscription config.
src/aux-records/AIController.spec.ts
Outdated
| @@ -1502,6 +1607,111 @@ describe('AIController', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it('should return success when allowedModels includes the model', async () => { | |||
| }); | ||
| }); | ||
|
|
||
| it('should return not_authorized error when allowedModels does not include the model', async () => { |
- Relocated test code from both "chat" and "chatStream"
fixes #328