AI class refactor #15760
AI class refactor #15760
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
|
|
||
| response = llmWrapper?.llm | ||
| ? await llmWrapper.run(inputs.prompt) | ||
| const llm = await pro.ai.getLLM(inputs.model) |
There was a problem hiding this comment.
getLLM performs the pro features checks that were previously being performed here. getLLM is designed to return an LLM if the user has access to one, and nothing if they don't.
| @@ -79,59 +60,4 @@ describe("rowProcessor utility", () => { | |||
| expect(fixAutoColumnSubType(schema)).toEqual(schema) | |||
| }) | |||
| }) | |||
|
|
|||
| describe("processAIColumns", () => { | |||
There was a problem hiding this comment.
What was being tested here is covered by more realistic tests of AI columns in `row.spec.ts.
| ) | ||
| if (rows && llmWrapper.llm) { | ||
| const llm = await pro.ai.getLLM() | ||
| if (rows && llm) { |
There was a problem hiding this comment.
It's nicer here now that we don't have to reach into the returned class and check that a property of it is not null (llmWrapper.llm from old diff) before we can use it.
| export type AIProvider = | ||
| | "OpenAI" | ||
| | "Anthropic" | ||
| | "AzureOpenAI" |
There was a problem hiding this comment.
"AzureOpenAI" was missing from this list, it is referenced in several places outside this file.
There was a problem hiding this comment.
Did the ts not complain about this missing?
There was a problem hiding this comment.
No, the typing in this area is a bit loose. This PR tightens it up a little, but there's likely still more we could do.
| export interface ProviderConfig { | ||
| provider: AIProvider | ||
| isDefault: boolean | ||
| isBudibaseAI?: boolean |
There was a problem hiding this comment.
This field is new, and much more explicitly labels a config as Budibase AI. We were previously inferring this from other fields. Budibase AI config is always enriched at runtime, so this won't be stored anywhere.
| @@ -17,4 +17,5 @@ export interface License { | |||
| plan: PurchasedPlan | |||
| billing?: Billing | |||
| testClockId?: string | |||
| tenantId?: string | |||
There was a problem hiding this comment.
| export type AIProvider = | ||
| | "OpenAI" | ||
| | "Anthropic" | ||
| | "AzureOpenAI" |
There was a problem hiding this comment.
Did the ts not complain about this missing?
|
Thanks for the detailed comments! |
Description
This change is mostly the set of companion changes required for the corresponding pro PR. The details of the changes are on that PR.
Pro PR: https://github.com/Budibase/budibase-pro/pull/364