Skip to content
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

[EA Forum only] updates to the criticism tips bot, temporarily pause showing the card to users #9765

Merged
merged 7 commits into from
Sep 20, 2024

Conversation

s-cheng
Copy link
Collaborator

@s-cheng s-cheng commented Sep 18, 2024

OpenAI is giving away free training tokens for fine-tuning GPT-4o, so I figured it's a good time to update the criticism tips bot. I decided to first test out just directly querying GPT-4o because that's less hassle than fine-tuning a new model, and it seemed to work great, so I did that instead.

So this PR has the following updates for the criticism tips bot:

  1. Replace querying the old fine-tuned model with querying gpt-4o-mini (which performed better than gpt-4o in my tests, and is cheaper)
  2. Temporarily comment out the user-facing part of this feature (the card), so that I can monitor the bot's performance on live data (via analytics events) for a while to make sure it's not terrible
  3. UI updates for the card, because the position shifted since it was originally deployed, and there's now a new card that may appear in a similar space ("Useful links")
  4. Rather that tracking dismissal of the card per post, we track dismissal of the card per user
Screenshot 2024-09-17 at 11 44 19 PM

┆Issue is synchronized with this Asana task by Unito

@@ -162,7 +162,7 @@ export const getPostMarketInfo = async (post: DbPost, context: ResolverContext):
void refreshMarketInfoInCache(post, context);
}

return { probability: cacheItem.probability, isResolved: cacheItem.isResolved, year: cacheItem.year, url: cacheItem.url };
return { probability: cacheItem.probability, isResolved: cacheItem.isResolved, year: cacheItem.year, url: cacheItem.url ?? '' };
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed an error here so I added this to fix it

@@ -133,24 +131,17 @@ function preprocessHtml(html: string): string {
return $.html();
}

export async function postToPrompt({template, post, promptSuffix, postBodyCache, markdownBody}: {
export async function postToPrompt({template, post, promptSuffix, postBodyCache}: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I previously made some changes to this function to work with the PostIsCriticismRequest, this basically undoes that

@s-cheng s-cheng marked this pull request as ready for review September 18, 2024 04:49
@s-cheng s-cheng requested a review from a team as a code owner September 18, 2024 04:49
@s-cheng s-cheng requested review from oetherington and removed request for a team September 18, 2024 04:49
*/
export async function postIsCriticism(post: PostIsCriticismRequest, currentUserId?: string): Promise<boolean> {
// Only run this on the EA Forum on production, since it costs money.
if (!isEAForum || !isProduction) return false
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to test this by removing the !isProduction condition - now that it's not using a fine-tuned model this will actually work with dev credentials :)

Copy link
Collaborator

@oetherington oetherington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM!

import { addField, dropField } from "./meta/utils"

export const up = async ({db}: MigrationContext) => {
await addField(db, Users, 'criticismTipsDismissed')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this migration also drop the field from the posts table?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally included that, but then I changed it because I figured it was safer to do it in a separate migration after deploying this? I think that's what we've done previously

top: -100,
height: '120%',
'@media (max-width: 1500px)': {
right: -335,
'@media (max-width: 1670px)': {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These breakpoints are used in a lot of places - maybe extract into a constant?

@oetherington oetherington assigned s-cheng and unassigned oetherington Sep 20, 2024
@s-cheng s-cheng merged commit 0a449f7 into master Sep 20, 2024
22 checks passed
@s-cheng s-cheng deleted the update-criticism-tips-bot branch September 20, 2024 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants