-
Notifications
You must be signed in to change notification settings - Fork 117
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
[LW] Second Recombee Hybrid Tab for Admins #9176
Conversation
@@ -177,13 +177,18 @@ const recombeeRequestHelpers = { | |||
}; | |||
}, | |||
|
|||
convertHybridToRecombeeArgs(hybridArgs: HybridRecombeeConfiguration, hybridArm: keyof typeof HYBRID_SCENARIO_MAP, filter?: string) { | |||
const { loadMore, userId, ...rest } = hybridArgs; | |||
convertHybridToRecombeeArgs(hybridArgs: HybridRecombeeConfiguration, hybridArm: 'first' | 'second', filter?: string) { |
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 we're making this more generic, then why not just 0 | 1
? Eliminates the translation on L187
|
||
const isConfigurable = hybridArm === 'second'; | ||
const clientConfig: Partial<Omit<HybridRecombeeConfiguration,"loadMore"|"userId">> = isConfigurable ? rest : {rotationRate: 0.1, rotationTime: 12}; | ||
const prevRecommIdIndex = isConfigurable ? 1 : 0; |
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.
Is this supposed to be flipped?
@@ -113,6 +113,7 @@ export interface RecombeeConfiguration { | |||
rotationTime?: number, | |||
booster?: string, | |||
filter?: string, | |||
hybridScenarios?: string[], |
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.
You can enforce that these are length of two by typing it as [string, string]
@@ -127,6 +128,7 @@ export interface RecombeeRecommendationArgs extends RecombeeConfiguration { | |||
} | |||
|
|||
export interface HybridRecombeeConfiguration { | |||
hybridScenarios: string[], |
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.
You can enforce that these are length of two by typing it as [string, string]
Production has live a recombee recommendation list that's 50-50 emulate hacker news algo + recombee:personal logic.
This PR adds a second tab for comparison that's 50-50 emulate hacker news + custom-for-lw-recombee-logic that they made for us (has some tweakable parameters too in Recombee admin config on their site)
┆Issue is synchronized with this Asana task by Unito