feat: aila instantiation includes option to specify a schema and categorisation approach #576
Conversation
…y doc for the current types to work
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Playwright test results
Details
Skipped testsNo persona › tests/auth.test.ts › authenticate through Clerk UI |
| @@ -218,26 +237,66 @@ export class AilaChat implements AilaChatService { | |||
| return applicableMessages; | |||
| } | |||
|
|
|||
| // #TODO this is the other part of the initial lesson state setting logic | |||
There was a problem hiding this comment.
Addresses this TODO about making the initial categorisation step something pluggable / specific to the type of document we are trying to create
| document: { | ||
| content: dbLessonPlan ?? {}, | ||
| schema: LessonPlanSchema, |
There was a problem hiding this comment.
This is the main starting point. We want to pass in a schema that we want to work with.
| document: { | ||
| content: dbLessonPlan ?? {}, | ||
| schema: LessonPlanSchema, | ||
| categorisationPlugin: (aila: AilaServices) => |
There was a problem hiding this comment.
And an optional categorisation plugin
stefl
changed the title
stefl
changed the title
| document: { content: {} }, | ||
| document: { | ||
| content: {}, | ||
| schema: LessonPlanSchema, |
There was a problem hiding this comment.
This is where we say what kind of document we want the Aila instance to deal with.
|
| ailaInstance.document.content = { | ||
| ...ailaInstance.document.content, | ||
| title: "Roman Britain", | ||
| subject: "history", | ||
| keyStage: "key-stage-2", | ||
| topic: "The Roman Empire", | ||
| }; |
There was a problem hiding this comment.
It would be useful to have a comment explaining this, as it seems to be overwriting the result of the test
| /** | ||
| * Ensures values are safe to use in enqueuePatch using Zod validation | ||
| * with a maximum recursion depth to prevent stack overflow | ||
| */ |
There was a problem hiding this comment.
This is starting to feel too specific to live in the AilaChat module. Do we need a module relating to initial state?
| } | ||
| } | ||
|
|
||
| return hasValidProperties ? result : undefined; |
There was a problem hiding this comment.
I think you can drop hasValidProperties by using Object.keys(result).length ? result : undefined
That also opens up more functional options to build the object like a map and filter - though that's a code style thing
| @@ -260,7 +323,10 @@ export class AilaChat implements AilaChatService { | |||
| if (!warning) { | |||
| return; | |||
| } | |||
| await this.enqueue({ type: "prompt", message: warning }); | |||
| // Optional "?" Necessary to avoid a "terminated" error | |||
There was a problem hiding this comment.
This is new to me. Are you saying that this can be undefined?
There was a problem hiding this comment.
Yes I was surprised too. The instance detaches when it's removed, and so this no longer exists, or something like that? This is just reinserting a comment that should have been there to match the others. No code change
|
|
||
| const log = aiLogger("aila"); | ||
|
|
||
| class TestDocument { |
There was a problem hiding this comment.
This file could do with a comment explaining what it does, as as far as I can tell it's defining a local class which isn't used in the codebase (yet?) and then testing it
| /** | ||
| * Get the appropriate plugin for the content | ||
| * Since we now only register one plugin, we just return the first one | ||
| */ |
There was a problem hiding this comment.
If we're only using one plugin, and it only works if there is one plugin, should we just support one plugin?
A bit of an embarrassing question, but what actually is a plugin? I can't find where it's being applied in the calling code
Description