-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: add an incrementing iteration number to the chat #263
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Playwright test resultsDetails Open report ↗︎ Skipped testsNo persona › tests/auth.test.ts › authenticate through Clerk UI |
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.
I haven't given it a poke at runtime, but this looks like a good change 👍
@@ -320,7 +334,7 @@ describe("Aila", () => { | |||
expect(ailaInstance.lesson.plan.title).toBe("Mocked Lesson Plan"); | |||
expect(ailaInstance.lesson.plan.subject).toBe("Mocked Subject"); | |||
expect(ailaInstance.lesson.plan.keyStage).toBe("key-stage-3"); | |||
}); | |||
}, 8000); |
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.
Do we still need these extended timeouts if we're using the mock categoriser?
@@ -94,6 +99,7 @@ export class AilaStreamHandler { | |||
await this.fetchChunkFromStream(); | |||
} catch (error) { | |||
await this._chat.generationFailed(error); | |||
throw error; |
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.
Nice find
public setPlan(plan: LooseLessonPlan) { | ||
this._plan = plan; | ||
} |
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.
I think that we could do with some better patterns for getters and setters. For just the plan, we have the class definition, setting the initial value in the initialiser, 2 setter methods and a getter
I assume we don't need the two setters, but it's indicative of the problem of not having a nice pattern
Something like parameter properties, but I think that would only work for non-destructured properties
Quality Gate passedIssues Measures |
🎉 This PR is included in version 1.13.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Description