Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

Commit

Permalink
feat(suggest): Add score field to suggestion (#119)
Browse files Browse the repository at this point in the history
Score indiciates the relative preference between suggestions, with higher
scores being preferred. This can allow Firefox and other clients of Merino to
make more intelligent decisions about what suggestions to include when many are
available.

Fixes #118
  • Loading branch information
mythmon authored Sep 2, 2021
1 parent 27e7702 commit 06556d8
Show file tree
Hide file tree
Showing 12 changed files with 182 additions and 10 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion merino-adm/src/remote_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use http::Uri;
use lazy_static::lazy_static;
use merino_settings::Settings;
use merino_suggest::{
SetupError, SuggestError, Suggestion, SuggestionProvider, SuggestionRequest, SuggestionResponse,
Proportion, SetupError, SuggestError, Suggestion, SuggestionProvider, SuggestionRequest,
SuggestionResponse,
};
use remote_settings_client::client::FileStorage;
use serde::Deserialize;
Expand Down Expand Up @@ -195,6 +196,7 @@ impl RemoteSettingsSuggester {
is_sponsored: !NON_SPONSORED_IAB_CATEGORIES
.contains(&adm_suggestion.iab_category.as_str()),
icon: icon_url,
score: Proportion::from(0.2),
});
for keyword in adm_suggestion.keywords {
suggestions.insert(keyword, merino_suggestion.clone());
Expand Down Expand Up @@ -365,6 +367,7 @@ mod tests {
provider: "test".to_string(),
is_sponsored: false,
icon: Uri::from_static("https://en.wikipedia.org/favicon.ico"),
score: Proportion::zero(),
}),
);
let rs_suggester = RemoteSettingsSuggester { suggestions };
Expand Down Expand Up @@ -404,6 +407,7 @@ mod tests {
provider: "test".to_string(),
is_sponsored: false,
icon: Uri::from_static("https://en.wikipedia.org/favicon.ico"),
score: Proportion::zero(),
}),
);
let rs_suggester = RemoteSettingsSuggester { suggestions };
Expand Down
1 change: 1 addition & 0 deletions merino-cache/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ uuid = "0.8"
http = "^0.2"
proptest = "^1"
fake = "2.4"
tokio = { version = "1", features = ["macros"] }
4 changes: 3 additions & 1 deletion merino-cache/src/redis/domain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl FromRedisValue for RedisTtl {
mod tests {
use anyhow::{anyhow, Result};
use http::Uri;
use merino_suggest::Suggestion;
use merino_suggest::{Proportion, Suggestion};
use proptest::prelude::*;
use redis::{FromRedisValue, ToRedisArgs};

Expand All @@ -120,6 +120,7 @@ mod tests {
provider: "One Inc.".to_string(),
is_sponsored: true,
icon: Uri::from_static("https://example.com/icon/one.png"),
score: Proportion::zero(),
}];

let mut serialized = SERIALIZATION_VERSION.to_vec();
Expand Down Expand Up @@ -187,6 +188,7 @@ mod tests {
provider: "One Inc.".to_string(),
is_sponsored: true,
icon: Uri::from_static("https://example.com/icon/one.png"),
score: Proportion::zero(),
}];

let val = RedisSuggestions(original_suggestions.clone()).to_redis_args();
Expand Down
7 changes: 4 additions & 3 deletions merino-cache/src/redis/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ where
#[cfg(test)]
mod test {
use http::Uri;
use merino_suggest::Suggestion;
use merino_suggest::{Proportion, Suggestion};

use super::*;

Expand Down Expand Up @@ -434,6 +434,7 @@ mod test {
provider: text.clone(),
is_sponsored: false,
icon: uri.clone(),
score: Proportion::zero(),
};
let suggestion2 = Suggestion {
id: 5678,
Expand Down Expand Up @@ -476,7 +477,7 @@ mod test {
.query_async::<redis::aio::ConnectionManager, String>(&mut redis_connection)
.await
.unwrap();
assert_eq!(res1, res2);
assert_eq!(res1, res2, "cached values should match");

// trying to write with an new lock should work and release the lock.
assert!(rlock
Expand All @@ -491,7 +492,7 @@ mod test {
.query_async::<redis::aio::ConnectionManager, String>(&mut redis_connection)
.await
.unwrap();
assert_ne!(res1, res2);
assert_ne!(res1, res2, "cached values should not match");

Ok(())
}
Expand Down
1 change: 1 addition & 0 deletions merino-integration-tests/src/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ async fn test_expected_fields(TestingTools { test_client, .. }: TestingTools) ->
"advertiser",
"is_sponsored",
"icon",
"score",
];
dbg!(&keys);
for expected_key in expected_keys {
Expand Down
4 changes: 3 additions & 1 deletion merino-suggest/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ use fake::{Fake, Faker};
use merino_settings::Settings;

use crate::{
SetupError, SuggestError, Suggestion, SuggestionProvider, SuggestionRequest, SuggestionResponse,
Proportion, SetupError, SuggestError, Suggestion, SuggestionProvider, SuggestionRequest,
SuggestionResponse,
};

/// A toy suggester to test the system.
Expand Down Expand Up @@ -49,6 +50,7 @@ impl<'a> SuggestionProvider<'a> for DebugProvider {
Ok(SuggestionResponse::new(vec![Suggestion {
title: json,
provider: "Merino::Debug".into(),
score: Proportion::zero(),
..Faker.fake()
}]))
}
Expand Down
140 changes: 140 additions & 0 deletions merino-suggest/src/domain.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
//! Datatypes to better represent the domain of Merino.

use anyhow::ensure;
use rand::distributions::{Distribution, Standard};
use serde::{de, Deserialize, Serialize};
use std::convert::{TryFrom, TryInto};
use std::fmt::Debug;

/// Represents a value from 0.0 to 1.0, inclusive. That is: a portion of
/// something that cannot be negative or exceed 100%.
///
/// Stored internally as a u32.
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Ord, PartialOrd, Hash)]
pub struct Proportion(u32);

impl Proportion {
/// The lowest value for a portion, corresponding to 0%.
pub fn zero() -> Self {
Proportion(0)
}

/// The highest value for a portion, corresponding to 100%.
pub fn one() -> Self {
Proportion(u32::MAX)
}

/// Converts a float value into a Proportion. Panics if the value is not
/// between zero and one.
///
/// This is not implemented using [`std::config::From`] because you cannot
/// implement both Try and TryFrom for the same pair of types, due to a
/// blanket `impl TryFor<T> for U where U: Try<T>`.
pub fn from<T>(v: T) -> Self
where
T: TryInto<Self>,
<T as TryInto<Self>>::Error: Debug,
{
v.try_into().unwrap()
}
}

macro_rules! impl_for_float {
($type: ty) => {
impl TryFrom<$type> for Proportion {
type Error = anyhow::Error;

fn try_from(v: $type) -> Result<Self, Self::Error> {
ensure!(!v.is_infinite(), "v cannot be infinite");
ensure!(v >= 0.0, "v must be positive");
ensure!(v <= 1.0, "v cannot be greater than 1");

Ok(Self((v * (u32::MAX as $type)) as u32))
}
}

impl From<Proportion> for $type {
fn from(portion: Proportion) -> $type {
(portion.0 as $type) / (u32::MAX as $type)
}
}

impl From<&Proportion> for $type {
fn from(portion: &Proportion) -> $type {
(portion.0 as $type) / (u32::MAX as $type)
}
}
};
}

impl_for_float!(f32);
impl_for_float!(f64);

impl Distribution<Proportion> for Standard {
fn sample<R: rand::Rng + ?Sized>(&self, rng: &mut R) -> Proportion {
Proportion(rng.gen())
}
}

impl Serialize for Proportion {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
serializer.serialize_f64(self.into())
}
}

impl<'de> Deserialize<'de> for Proportion {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
/// Visitor for deserializing a Proportion
struct Visitor;

impl<'de> de::Visitor<'de> for Visitor {
type Value = Proportion;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(formatter, "value between 0.0 and 1.0")
}

fn visit_i64<E>(self, v: i64) -> Result<Self::Value, E>
where
E: de::Error,
{
if v >= 0 {
self.visit_u64(v as u64)
} else {
Err(de::Error::invalid_value(de::Unexpected::Signed(v), &self))
}
}

// u8, u16, and u32 delegate to this
fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
where
E: de::Error,
{
if v == 0 {
Ok(Proportion::zero())
} else if v == 1 {
Ok(Proportion::one())
} else {
Err(de::Error::invalid_value(de::Unexpected::Unsigned(v), &self))
}
}

// f32 delegates to this
fn visit_f64<E>(self, v: f64) -> Result<Self::Value, E>
where
E: de::Error,
{
v.try_into()
.map_err(|_err| de::Error::invalid_value(de::Unexpected::Float(v), &self))
}
}

deserializer.deserialize_any(Visitor)
}
}
13 changes: 12 additions & 1 deletion merino-suggest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

mod debug;
pub mod device_info;
mod domain;
mod multi;
mod wikifruit;

Expand All @@ -13,6 +14,7 @@ use std::hash::Hash;
use std::ops::Range;
use std::time::Duration;

use crate::device_info::DeviceInfo;
use async_trait::async_trait;
use fake::{
faker::{
Expand All @@ -27,7 +29,7 @@ use serde_with::{serde_as, DisplayFromStr};
use thiserror::Error;

pub use crate::debug::DebugProvider;
use crate::device_info::DeviceInfo;
pub use crate::domain::Proportion;
pub use crate::multi::Multi;
pub use crate::wikifruit::WikiFruit;

Expand Down Expand Up @@ -195,6 +197,14 @@ pub struct Suggestion {
/// The URL of the icon to show along side this suggestion.
#[serde_as(as = "DisplayFromStr")]
pub icon: Uri,

/// A value used to compare suggestions. When choosing a suggestion to show
/// the user, higher scored suggestions are preferred. Should range from 0.0
/// to 1.0.
///
/// Note that Firefox uses a static value of 0.2 for Remote Settings
/// provided suggestions.
pub score: Proportion,
}

impl<'a, F> fake::Dummy<F> for Suggestion {
Expand All @@ -209,6 +219,7 @@ impl<'a, F> fake::Dummy<F> for Suggestion {
provider: Words(2..4).fake_with_rng::<Vec<String>, R>(rng).join(" "),
is_sponsored: rng.gen(),
icon: fake_example_url(rng),
score: rng.gen(),
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion merino-suggest/src/wikifruit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use http::Uri;
use merino_settings::Settings;

use crate::{
SetupError, SuggestError, Suggestion, SuggestionProvider, SuggestionRequest, SuggestionResponse,
domain::Proportion, SetupError, SuggestError, Suggestion, SuggestionProvider,
SuggestionRequest, SuggestionResponse,
};

/// A toy suggester to test the system.
Expand Down Expand Up @@ -56,6 +57,7 @@ impl<'a> SuggestionProvider<'a> for WikiFruit {
provider: "Merino::WikiFruit".to_string(),
is_sponsored: false,
icon: Uri::from_static("https://en.wikipedia.org/favicon.ico"),
score: Proportion::zero(),
}),
"banana" => Some(Suggestion {
id: 1,
Expand All @@ -67,6 +69,7 @@ impl<'a> SuggestionProvider<'a> for WikiFruit {
provider: "Merino::WikiFruit".to_string(),
is_sponsored: false,
icon: Uri::from_static("https://en.wikipedia.org/favicon.ico"),
score: Proportion::zero(),
}),
"cherry" => Some(Suggestion {
id: 1,
Expand All @@ -78,6 +81,7 @@ impl<'a> SuggestionProvider<'a> for WikiFruit {
provider: "Merino::WikiFruit".to_string(),
is_sponsored: false,
icon: Uri::from_static("https://en.wikipedia.org/favicon.ico"),
score: Proportion::zero(),
}),
_ => None,
};
Expand Down
2 changes: 2 additions & 0 deletions merino-web/src/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ impl<'a> Serialize for SuggestionWrapper<'a> {
is_sponsored: bool,
icon: String,
advertiser: &'a str,
score: f32,
}

let provider = &self.0.provider;
Expand All @@ -176,6 +177,7 @@ impl<'a> Serialize for SuggestionWrapper<'a> {
is_sponsored: self.0.is_sponsored,
icon: self.0.icon.to_string(),
advertiser: provider,
score: self.0.score.into(),
};

generated.serialize(serializer)
Expand Down
4 changes: 4 additions & 0 deletions merino/src/docs/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ following keys:
small square image, suitable to be included inline with the text, such as a
site's favicon.
- `score` - A value between 0.0 and 1.0 used to compare suggestions. When
choosing a suggestion to show the user, higher scored suggestions are
preferred.
### Response Headers
Responses will carry standard HTTP caching headers that indicate the validity of
Expand Down

0 comments on commit 06556d8

Please sign in to comment.