-
Notifications
You must be signed in to change notification settings - Fork 333
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
chore: remove discussion prompt from summary #9711
chore: remove discussion prompt from summary #9711
Conversation
const hasMoreThanOneReflection = | ||
(reflectionGroups?.length && reflectionGroups.length > 1) || | ||
reflectionGroups?.some((group) => group.reflections.length > 1) | ||
if (!hasMoreThanOneReflection || organization.featureFlags.noAISummary) return null |
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.
If there's more than one reflection, we'll always create the meeting summary unless noAISummary
is true, so we don't need to check if there's a discussion summary or reflection group summary (which is now removed anyway)
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.
Did not test yet
const hasDiscussionSummary = !!stages?.some((stage) => stage.discussion?.summary) | ||
const hasOpenAISummary = hasTopicSummary || hasDiscussionSummary | ||
const hasMoreThanOneReflection = reflections.length > 1 | ||
const hasOpenAISummary = hasMoreThanOneReflection || !organization.featureFlags.noAISummary |
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.
-1 this should be &&
, shouldn't it?
) | ||
const [fullSummary, fullQuestion] = await Promise.all([ | ||
manager.getSummary(reflectionTextByGroupId), | ||
const [fullQuestion] = await Promise.all([ |
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.
-1 No need to wrap a single call in Promise.all([...])
Fix #9710
Demo: https://www.loom.com/share/728f893c2e054fbbad82f849e89ffeaa
The AI Summary for the reflection group is no longer being used but I don't want to remove it now as it looks like we're migrating that table from RethinkDB to PG. I've created an issue: #9712
To test